FrankChen021 commented on code in PR #19880:
URL: https://github.com/apache/druid/pull/19880#discussion_r3714755641
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentArchiverTest.java:
##########
@@ -100,7 +99,7 @@ public String getArchiveBaseKey()
.size(0)
.build();
- @BeforeClass
+
public static void setUpStatic()
Review Comment:
Fixed in 7f63dfa376: setUpStatic now has @BeforeAll.
S3DataSegmentArchiverTest passed all 4 tests.
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentMoverTest.java:
##########
@@ -211,30 +214,33 @@
), ImmutableMap.of("bucket", "DOES NOT EXIST", "baseKey", "baseKey"));
}
- @Test(expected = SegmentLoadingException.class)
+ @Test
public void testFailsToMoveMissing() throws Exception
{
MockAmazonS3Client mockS3Client = new MockAmazonS3Client();
S3DataSegmentMover mover = new S3DataSegmentMover(
Suppliers.ofInstance(mockS3Client),
new S3DataSegmentPusherConfig()
);
- mover.move(new DataSegment(
- "test",
- Intervals.of("2013-01-01/2013-01-02"),
- "1",
- ImmutableMap.of(
- "key",
-
"baseKey/test/2013-01-01T00:00:00.000Z_2013-01-02T00:00:00.000Z/1/0/index.zip",
- "bucket",
- "DOES NOT EXIST"
- ),
- ImmutableList.of("dim1", "dim1"),
- ImmutableList.of("metric1", "metric2"),
- NoneShardSpec.instance(),
- 0,
- 1
- ), ImmutableMap.of("bucket", "DOES NOT EXIST", "baseKey", "baseKey2"));
+ Assertions.assertThrows(
+ SegmentLoadingException.class,
+ () -> mover.move(new DataSegment(
+ "test",
+ Intervals.of("2013-01-01/2013-01-02"),
+ "1",
+ ImmutableMap.of(
+ "key",
+
"baseKey/test/2013-01-01T00:00:00.000Z_2013-01-02T00:00:00.000Z/1/0/index.zip",
+ "bucket",
+ "DOES NOT EXIST"
+ ),
+ ImmutableList.of("dim1", "dim1"),
+ ImmutableList.of("metric1", "metric2"),
+ NoneShardSpec.instance(),
+ 0,
+ 1
+ ), ImmutableMap.of("bucket", "DOES NOT EXIST", "baseKey", "baseKey2"))
Review Comment:
Evaluated as baseline: this deprecated DataSegment constructor already
existed on master before this JUnit migration. This PR does not alter it, so
deprecated API cleanup is separate scope and is left unchanged.
##########
extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/ServerSideEncryptingAmazonS3Test.java:
##########
@@ -233,13 +236,13 @@
transferManagerField.setAccessible(true);
Object transferManager = transferManagerField.get(s3);
- Assert.assertNull("TransferManager should be null when disabled",
transferManager);
+ Assertions.assertNull(transferManager, "TransferManager should be null
when disabled");
}
@Test
public void testBuilder() throws IOException
{
- File testFile = temporaryFolder.newFile("test-builder.txt");
+ File testFile = File.createTempFile("test-builder", ".txt",
temporaryFolder);
Review Comment:
Fixed in 7f63dfa376 by removing the unused testFile local. The two focused
S3 test classes passed all 14 tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
