FrankChen021 commented on code in PR #19926:
URL: https://github.com/apache/druid/pull/19926#discussion_r3739920056
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/ShardSpecsTest.java:
##########
@@ -84,7 +84,7 @@ public void testShardSpecSelectionWithNullPartitionDimension()
ShardSpec spec4 =
shardSpecs.getShardSpec(Intervals.of("2014-01-01T00:00:00.000Z/2014-01-02T00:00:00.000Z"),
row2);
ShardSpec spec5 =
shardSpecs.getShardSpec(Intervals.of("2014-01-01T00:00:00.000Z/2014-01-02T00:00:00.000Z"),
row3);
- Assert.assertSame(true, spec3 == spec4);
- Assert.assertSame(false, spec3 == spec5);
+ Assertions.assertSame(true, spec3 == spec4);
+ Assertions.assertSame(false, spec3 == spec5);
Review Comment:
Fixed in commit a5780d1c93: replaced assertSame(true, ...) and
assertSame(false, ...) with assertSame(spec3, spec4) and assertNotSame(spec3,
spec5), so the test directly checks ShardSpec identity.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/IngestionTestBase.java:
##########
@@ -468,7 +475,8 @@ public ListenableFuture<TaskStatus> run(Task task)
lockbox.add(task);
taskStorage.insert(task, TaskStatus.running(task.getId()));
taskActionClient = createActionClient(task);
- taskReportsFile = temporaryFolder.newFile(
+ taskReportsFile = new File(
+ temporaryFolder,
StringUtils.format("ingestionTestBase-%s.json",
System.currentTimeMillis())
);
Review Comment:
Evaluated and intentionally left the production path unchanged. The parallel
test path writes its report directly to reportsFile in
AbstractParallelIndexSupervisorTaskTest; taskReportsFile belongs to the
separate legacy TestTaskRunner helper. The focused
CompactionTaskParallelRunTest and related tests pass.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/ShardSpecsTest.java:
##########
@@ -84,7 +84,7 @@ public void testShardSpecSelectionWithNullPartitionDimension()
ShardSpec spec4 =
shardSpecs.getShardSpec(Intervals.of("2014-01-01T00:00:00.000Z/2014-01-02T00:00:00.000Z"),
row2);
ShardSpec spec5 =
shardSpecs.getShardSpec(Intervals.of("2014-01-01T00:00:00.000Z/2014-01-02T00:00:00.000Z"),
row3);
- Assert.assertSame(true, spec3 == spec4);
- Assert.assertSame(false, spec3 == spec5);
+ Assertions.assertSame(true, spec3 == spec4);
+ Assertions.assertSame(false, spec3 == spec5);
Review Comment:
Fixed in commit a5780d1c93: replaced assertSame(true, ...) and
assertSame(false, ...) with assertSame(spec3, spec4) and assertNotSame(spec3,
spec5), so the test directly checks ShardSpec identity.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/IngestionTestBase.java:
##########
@@ -468,7 +475,8 @@ public ListenableFuture<TaskStatus> run(Task task)
lockbox.add(task);
taskStorage.insert(task, TaskStatus.running(task.getId()));
taskActionClient = createActionClient(task);
- taskReportsFile = temporaryFolder.newFile(
+ taskReportsFile = new File(
+ temporaryFolder,
StringUtils.format("ingestionTestBase-%s.json",
System.currentTimeMillis())
);
Review Comment:
Evaluated and intentionally left the production path unchanged. The parallel
test path writes its report directly to reportsFile in
AbstractParallelIndexSupervisorTaskTest; taskReportsFile belongs to the
separate legacy TestTaskRunner helper. The focused
CompactionTaskParallelRunTest and related tests pass.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/IngestionTestBase.java:
##########
@@ -468,7 +475,8 @@ public ListenableFuture<TaskStatus> run(Task task)
lockbox.add(task);
taskStorage.insert(task, TaskStatus.running(task.getId()));
taskActionClient = createActionClient(task);
- taskReportsFile = temporaryFolder.newFile(
+ taskReportsFile = new File(
+ temporaryFolder,
StringUtils.format("ingestionTestBase-%s.json",
System.currentTimeMillis())
);
Review Comment:
Evaluated and intentionally left the production path unchanged. The parallel
test path writes its report directly to reportsFile in
AbstractParallelIndexSupervisorTaskTest; taskReportsFile belongs to the
separate legacy TestTaskRunner helper. The focused
CompactionTaskParallelRunTest and related tests pass.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/ShardSpecsTest.java:
##########
@@ -84,7 +84,7 @@ public void testShardSpecSelectionWithNullPartitionDimension()
ShardSpec spec4 =
shardSpecs.getShardSpec(Intervals.of("2014-01-01T00:00:00.000Z/2014-01-02T00:00:00.000Z"),
row2);
ShardSpec spec5 =
shardSpecs.getShardSpec(Intervals.of("2014-01-01T00:00:00.000Z/2014-01-02T00:00:00.000Z"),
row3);
- Assert.assertSame(true, spec3 == spec4);
- Assert.assertSame(false, spec3 == spec5);
+ Assertions.assertSame(true, spec3 == spec4);
+ Assertions.assertSame(false, spec3 == spec5);
Review Comment:
Fixed in commit a5780d1c93: replaced assertSame(true, ...) and
assertSame(false, ...) with assertSame(spec3, spec4) and assertNotSame(spec3,
spec5), so the test directly checks ShardSpec identity.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/AbstractParallelIndexSupervisorTaskTest.java:
##########
@@ -167,11 +165,27 @@
private static final Logger LOG = new
Logger(AbstractParallelIndexSupervisorTaskTest.class);
- @Rule
- public final TemporaryFolder temporaryFolder = new TemporaryFolder();
+ protected final File temporaryFolder = FileUtils.createTempDir();
Review Comment:
Fixed in commit a5780d1c93: renamed the subclass field to
parallelTemporaryFolder and updated its uses, preserving its separate cleanup
lifecycle while removing the field shadowing.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/AbstractParallelIndexSupervisorTaskTest.java:
##########
@@ -167,11 +165,27 @@
private static final Logger LOG = new
Logger(AbstractParallelIndexSupervisorTaskTest.class);
- @Rule
- public final TemporaryFolder temporaryFolder = new TemporaryFolder();
+ protected final File temporaryFolder = FileUtils.createTempDir();
Review Comment:
Fixed in commit a5780d1c93: renamed the subclass field to
parallelTemporaryFolder and updated its uses, preserving its separate cleanup
lifecycle while removing the field shadowing.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/AbstractParallelIndexSupervisorTaskTest.java:
##########
@@ -167,11 +165,27 @@
private static final Logger LOG = new
Logger(AbstractParallelIndexSupervisorTaskTest.class);
- @Rule
- public final TemporaryFolder temporaryFolder = new TemporaryFolder();
+ protected final File temporaryFolder = FileUtils.createTempDir();
Review Comment:
Fixed in commit a5780d1c93: renamed the subclass field to
parallelTemporaryFolder and updated its uses, preserving its separate cleanup
lifecycle while removing the field shadowing.
--
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]
