You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "FrankChen021 (via GitHub)" <gi...@apache.org> on 2026/08/07 12:15:40 UTC
[PR] test: migrate second extensions-contrib batch to JUnit 5 (druid)
FrankChen021 opened a new pull request, #19915:
URL: https://github.com/apache/druid/pull/19915
Part of [#13948](https://github.com/apache/druid/issues/13948).
## Summary
This PR migrates the second independent `extensions-contrib` batch from JUnit 4 to JUnit 5:
- `moving-average-query`
- `aliyun-oss-extensions`
- `rabbit-stream-indexing-service`
- `opentsdb-emitter`
The module POMs remove JUnit 4 and Hamcrest test dependencies, and the test sources use Jupiter annotations, assertions, and parameterization. `MovingAverageQueryTest` now owns its small no-op scheduler fixture instead of loading the JUnit-4-based `QueryStackTests` class.
This branch is based directly on `master`; it does not stack on [#19908](https://github.com/apache/druid/pull/19908) and contains no shared SQL/processing fixture changes. It can be merged independently. The shared fixtures are intentionally owned only by #19908.
Related migration PRs: [#19908](https://github.com/apache/druid/pull/19908), [#19875](https://github.com/apache/druid/pull/19875), [#19876](https://github.com/apache/druid/pull/19876), [#19877](https://github.com/apache/druid/pull/19877), [#19878](https://github.com/apache/druid/pull/19878), [#19879](https://github.com/apache/druid/pull/19879), [#19880](https://github.com/apache/druid/pull/19880), [#19881](https://github.com/apache/druid/pull/19881), and [#19882](https://github.com/apache/druid/pull/19882).
## Validation
- `test-compile` passed for all four modules and their required reactor dependencies.
- Focused tests passed: moving-average (65), Aliyun OSS (68), Rabbit stream (39), and OpenTSDB (11).
- Checkstyle passed with 0 violations for all four modules.
- SpotBugs passed with 0 bugs/errors for all four modules.
- No JUnit 4 or Hamcrest source imports or direct POM dependencies remain in these modules.
- `git diff --check` passed.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test: migrate second extensions-contrib batch to JUnit 5 (druid)
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #19915:
URL: https://github.com/apache/druid/pull/19915#discussion_r3736351798
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTaskLogsTest.java:
##########
@@ -424,7 +423,8 @@ private List<Grant> testPushInternal(boolean disableAcl, String ownerId, String
OssTaskLogs taskLogs = new OssTaskLogs(ossClient, config, inputDataConfig, timeSupplier);
String taskId = "index_test-datasource_2019-06-18T13:30:28.887Z";
- File logFile = tempFolder.newFile("test_log_file");
+ File logFile = new File(tempFolder, "test_log_file");
+ Assertions.assertTrue(logFile.createNewFile());
Review Comment:
Repository style requires using `final` for variables that are not reassigned (AGENTS.md:40). `taskId` and `logFile` are not reassigned and can be `final`.
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTimestampVersionedDataFinderTest.java:
##########
@@ -102,7 +102,7 @@ public void testMissing()
EasyMock.verify(oss);
- Assert.assertEquals(null, latest);
+ Assertions.assertEquals(null, latest);
Review Comment:
Prefer `Assertions.assertNull(latest)` over `assertEquals(null, latest)` for clearer intent and better failure messages when this assertion fails.
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssDataSegmentPusherTest.java:
##########
@@ -91,7 +90,7 @@ private void testPushInternal(boolean useUniquePath, String matcher) throws Exce
OssDataSegmentPusher pusher = new OssDataSegmentPusher(client, config);
// Create a mock segment on disk
- File tmp = tempFolder.newFile("version.bin");
+ File tmp = new File(tempFolder, "version.bin");
Review Comment:
Repository style requires using `final` for variables that are not reassigned (AGENTS.md:40). `tmp` is not reassigned and can be `final`.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test: migrate second extensions-contrib batch to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19915:
URL: https://github.com/apache/druid/pull/19915#discussion_r3736467469
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssDataSegmentPusherTest.java:
##########
@@ -91,7 +90,7 @@ private void testPushInternal(boolean useUniquePath, String matcher) throws Exce
OssDataSegmentPusher pusher = new OssDataSegmentPusher(client, config);
// Create a mock segment on disk
- File tmp = tempFolder.newFile("version.bin");
+ File tmp = new File(tempFolder, "version.bin");
Review Comment:
Fixed in commit `8a87f7204d`: made `tmp` `final`, satisfying AGENTS.md. The declaration is part of the modified JUnit 5 `@TempDir` migration line, so this is an in-scope style defect. Focused Aliyun Maven validation passed: compilation, checkstyle, and 13 tests (2 `OssDataSegmentPusherTest` + 11 `OssTaskLogsTest`).
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTaskLogsTest.java:
##########
@@ -424,7 +423,8 @@ private List<Grant> testPushInternal(boolean disableAcl, String ownerId, String
OssTaskLogs taskLogs = new OssTaskLogs(ossClient, config, inputDataConfig, timeSupplier);
String taskId = "index_test-datasource_2019-06-18T13:30:28.887Z";
- File logFile = tempFolder.newFile("test_log_file");
+ File logFile = new File(tempFolder, "test_log_file");
+ Assertions.assertTrue(logFile.createNewFile());
Review Comment:
Fixed in commit `8a87f7204d`: made `taskId` and `logFile` `final`, satisfying AGENTS.md. The modified `logFile` declaration is part of the JUnit 5 `@TempDir` migration; the earlier reply overlooked that line-level change. Focused Aliyun Maven validation passed: compilation, checkstyle, and 13 tests (2 `OssDataSegmentPusherTest` + 11 `OssTaskLogsTest`).
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test: migrate second extensions-contrib batch to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19915:
URL: https://github.com/apache/druid/pull/19915#discussion_r3736505960
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTimestampVersionedDataFinderTest.java:
##########
@@ -102,7 +102,7 @@ public void testMissing()
EasyMock.verify(oss);
- Assert.assertEquals(null, latest);
+ Assertions.assertEquals(null, latest);
Review Comment:
Classified as pre-existing/context-only and outdated. Base commit `fe4377a4` already used `Assert.assertEquals(null, latest)` at this assertion; the migration only changed it to `Assertions.assertEquals(null, latest)`. This is a clarity improvement rather than a defect introduced by the JUnit 5 migration, so no code change is included.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test: migrate second extensions-contrib batch to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 merged PR #19915:
URL: https://github.com/apache/druid/pull/19915
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test: migrate second extensions-contrib batch to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19915:
URL: https://github.com/apache/druid/pull/19915#discussion_r3736710428
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/data/input/aliyun/OssInputSourceTest.java:
##########
@@ -538,20 +543,20 @@
InputSourceReader reader = inputSource.reader(
someSchema,
new CsvInputFormat(ImmutableList.of("time", "dim1", "dim2"), "|", false, null, 0, null),
- temporaryFolder.newFolder()
+ newFolder(temporaryFolder, "junit")
);
final InputStats inputStats = new InputStatsImpl();
CloseableIterator<InputRow> iterator = reader.read(inputStats);
while (iterator.hasNext()) {
InputRow nextRow = iterator.next();
- Assert.assertEquals(NOW, nextRow.getTimestamp());
- Assert.assertEquals("hello", nextRow.getDimension("dim1").get(0));
- Assert.assertEquals("world", nextRow.getDimension("dim2").get(0));
+ Assertions.assertEquals(NOW, nextRow.getTimestamp());
+ Assertions.assertEquals("hello", nextRow.getDimension("dim1").get(0));
+ Assertions.assertEquals("world", nextRow.getDimension("dim2").get(0));
}
- Assert.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes());
+ Assertions.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes());
Review Comment:
Fixed in commit `c4361e8ffb` by widening the operand before multiplication (`2L * CONTENT.length`), preventing the int overflow identified by CodeQL. The focused Aliyun OSS compilation, Checkstyle, and test validation passed. Resolving this fixed finding.
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/data/input/aliyun/OssInputSourceTest.java:
##########
@@ -585,20 +590,20 @@
InputSourceReader reader = inputSource.reader(
someSchema,
new CsvInputFormat(ImmutableList.of("time", "dim1", "dim2"), "|", false, null, 0, null),
- temporaryFolder.newFolder()
+ newFolder(temporaryFolder, "junit")
);
final InputStats inputStats = new InputStatsImpl();
CloseableIterator<InputRow> iterator = reader.read(inputStats);
while (iterator.hasNext()) {
InputRow nextRow = iterator.next();
- Assert.assertEquals(NOW, nextRow.getTimestamp());
- Assert.assertEquals("hello", nextRow.getDimension("dim1").get(0));
- Assert.assertEquals("world", nextRow.getDimension("dim2").get(0));
+ Assertions.assertEquals(NOW, nextRow.getTimestamp());
+ Assertions.assertEquals("hello", nextRow.getDimension("dim1").get(0));
+ Assertions.assertEquals("world", nextRow.getDimension("dim2").get(0));
}
- Assert.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes());
+ Assertions.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes());
Review Comment:
Fixed in commit `c4361e8ffb` by widening the operand before multiplication (`2L * CONTENT.length`), preventing the int overflow identified by CodeQL. The focused Aliyun OSS compilation, Checkstyle, and test validation passed. Resolving this fixed finding.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test: migrate second extensions-contrib batch to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19915:
URL: https://github.com/apache/druid/pull/19915#discussion_r3740241685
##########
extensions-contrib/moving-average-query/src/test/java/org/apache/druid/query/movingaverage/MovingAverageQueryTest.java:
##########
@@ -102,9 +101,17 @@
/**
* Base class for implementing MovingAverageQuery tests
*/
-@RunWith(Parameterized.class)
+@ParameterizedClass(name = "{0}")
+@MethodSource("data")
public class MovingAverageQueryTest extends InitializedNullHandlingTest
{
+ private static final QueryScheduler NOOP_SCHEDULER = new QueryScheduler(
Review Comment:
It's OK to define a new scheduler here intead of depending on the one in other modules. This also eliminates the extra dependencies across test modules.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test: migrate second extensions-contrib batch to JUnit 5 (druid)
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #19915:
URL: https://github.com/apache/druid/pull/19915#discussion_r3736014417
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/data/input/aliyun/OssInputSourceTest.java:
##########
@@ -538,20 +543,20 @@
InputSourceReader reader = inputSource.reader(
someSchema,
new CsvInputFormat(ImmutableList.of("time", "dim1", "dim2"), "|", false, null, 0, null),
- temporaryFolder.newFolder()
+ newFolder(temporaryFolder, "junit")
);
final InputStats inputStats = new InputStatsImpl();
CloseableIterator<InputRow> iterator = reader.read(inputStats);
while (iterator.hasNext()) {
InputRow nextRow = iterator.next();
- Assert.assertEquals(NOW, nextRow.getTimestamp());
- Assert.assertEquals("hello", nextRow.getDimension("dim1").get(0));
- Assert.assertEquals("world", nextRow.getDimension("dim2").get(0));
+ Assertions.assertEquals(NOW, nextRow.getTimestamp());
+ Assertions.assertEquals("hello", nextRow.getDimension("dim1").get(0));
+ Assertions.assertEquals("world", nextRow.getDimension("dim2").get(0));
}
- Assert.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes());
+ Assertions.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes());
Review Comment:
## CodeQL / Result of multiplication cast to wider type
Potential overflow in [int multiplication](1) before it is converted to long by use in an invocation context.
[Show more details](https://github.com/apache/druid/security/code-scanning/11687)
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/data/input/aliyun/OssInputSourceTest.java:
##########
@@ -585,20 +590,20 @@
InputSourceReader reader = inputSource.reader(
someSchema,
new CsvInputFormat(ImmutableList.of("time", "dim1", "dim2"), "|", false, null, 0, null),
- temporaryFolder.newFolder()
+ newFolder(temporaryFolder, "junit")
);
final InputStats inputStats = new InputStatsImpl();
CloseableIterator<InputRow> iterator = reader.read(inputStats);
while (iterator.hasNext()) {
InputRow nextRow = iterator.next();
- Assert.assertEquals(NOW, nextRow.getTimestamp());
- Assert.assertEquals("hello", nextRow.getDimension("dim1").get(0));
- Assert.assertEquals("world", nextRow.getDimension("dim2").get(0));
+ Assertions.assertEquals(NOW, nextRow.getTimestamp());
+ Assertions.assertEquals("hello", nextRow.getDimension("dim1").get(0));
+ Assertions.assertEquals("world", nextRow.getDimension("dim2").get(0));
}
- Assert.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes());
+ Assertions.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes());
Review Comment:
## CodeQL / Result of multiplication cast to wider type
Potential overflow in [int multiplication](1) before it is converted to long by use in an invocation context.
[Show more details](https://github.com/apache/druid/security/code-scanning/11688)
##########
extensions-contrib/rabbit-stream-indexing-service/src/test/java/org/apache/druid/indexing/rabbitstream/RabbitStreamIndexTaskTuningConfigTest.java:
##########
@@ -61,22 +56,22 @@
TuningConfig.class
);
- Assert.assertNull(config.getBasePersistDirectory());
- Assert.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
- Assert.assertEquals(150000, config.getMaxRowsInMemory());
- Assert.assertEquals(5_000_000, config.getMaxRowsPerSegment().intValue());
- Assert.assertEquals(new Period("PT10M"), config.getIntermediatePersistPeriod());
- Assert.assertEquals(0, config.getMaxPendingPersists());
- // Assert.assertEquals(IndexSpec.getDefault(), config.getIndexSpec());
- Assert.assertEquals(false, config.isReportParseExceptions());
- Assert.assertEquals(Duration.ofMinutes(15).toMillis(), config.getHandoffConditionTimeout());
-
- Assert.assertNull(config.getRecordBufferSizeConfigured());
- Assert.assertEquals(10000, config.getRecordBufferSizeOrDefault(1_000_000_000));
- Assert.assertEquals(5000, config.getRecordBufferOfferTimeout());
-
- Assert.assertFalse(config.isSkipSequenceNumberAvailabilityCheck());
- Assert.assertFalse(config.isResetOffsetAutomatically());
+ Assertions.assertNull(config.getBasePersistDirectory());
+ Assertions.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
+ Assertions.assertEquals(150000, config.getMaxRowsInMemory());
+ Assertions.assertEquals(5_000_000, config.getMaxRowsPerSegment().intValue());
+ Assertions.assertEquals(new Period("PT10M"), config.getIntermediatePersistPeriod());
+ Assertions.assertEquals(0, config.getMaxPendingPersists());
Review Comment:
## CodeQL / Deprecated method or constructor invocation
Invoking [SeekableStreamIndexTaskTuningConfig.getMaxPendingPersists](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/11690)
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssDataSegmentMoverTest.java:
##########
@@ -150,27 +154,29 @@
), ImmutableMap.of("bucket", "DOES NOT EXIST", "baseKey", "baseKey"));
}
- @Test(expected = SegmentLoadingException.class)
- public void testFailsToMoveMissing() throws Exception
+ @Test
+ public void testFailsToMoveMissing()
{
- MockClient client = new MockClient();
- OssDataSegmentMover mover = new OssDataSegmentMover(Suppliers.ofInstance(client), new OssStorageConfig());
- 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"));
+ assertThrows(SegmentLoadingException.class, () -> {
+ MockClient client = new MockClient();
+ OssDataSegmentMover mover = new OssDataSegmentMover(Suppliers.ofInstance(client), new OssStorageConfig());
+ 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:
## CodeQL / Deprecated method or constructor invocation
Invoking [DataSegment.DataSegment](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/11689)
##########
extensions-contrib/rabbit-stream-indexing-service/src/test/java/org/apache/druid/indexing/rabbitstream/RabbitStreamIndexTaskTuningConfigTest.java:
##########
@@ -106,18 +101,18 @@
TuningConfig.class
);
- Assert.assertNull(config.getBasePersistDirectory());
- Assert.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
- Assert.assertEquals(100, config.getMaxRowsInMemory());
- Assert.assertEquals(100, config.getMaxRowsPerSegment().intValue());
- Assert.assertEquals(new Period("PT1H"), config.getIntermediatePersistPeriod());
- Assert.assertEquals(100, config.getMaxPendingPersists());
- Assert.assertTrue(config.isReportParseExceptions());
- Assert.assertEquals(100, config.getHandoffConditionTimeout());
- Assert.assertEquals(1000, (int) config.getRecordBufferSizeConfigured());
- Assert.assertEquals(1000, config.getRecordBufferSizeOrDefault(1_000_000_000));
- Assert.assertEquals(500, config.getRecordBufferOfferTimeout());
- Assert.assertFalse(config.isResetOffsetAutomatically());
+ Assertions.assertNull(config.getBasePersistDirectory());
+ Assertions.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
+ Assertions.assertEquals(100, config.getMaxRowsInMemory());
+ Assertions.assertEquals(100, config.getMaxRowsPerSegment().intValue());
+ Assertions.assertEquals(new Period("PT1H"), config.getIntermediatePersistPeriod());
+ Assertions.assertEquals(100, config.getMaxPendingPersists());
Review Comment:
## CodeQL / Deprecated method or constructor invocation
Invoking [SeekableStreamIndexTaskTuningConfig.getMaxPendingPersists](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/11691)
##########
extensions-contrib/rabbit-stream-indexing-service/src/test/java/org/apache/druid/indexing/rabbitstream/supervisor/RabbitStreamSupervisorTuningConfigTest.java:
##########
@@ -134,22 +129,22 @@
TuningConfig.class)),
TuningConfig.class);
- Assert.assertNull(config.getBasePersistDirectory());
- Assert.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
- Assert.assertEquals(100, config.getMaxRowsInMemory());
- Assert.assertEquals(100, config.getMaxRowsPerSegment().intValue());
- Assert.assertEquals(new Period("PT1H"), config.getIntermediatePersistPeriod());
- Assert.assertEquals(100, config.getMaxPendingPersists());
- Assert.assertEquals(true, config.isReportParseExceptions());
- Assert.assertEquals(100, config.getHandoffConditionTimeout());
- Assert.assertEquals(12, (int) config.getWorkerThreads());
- Assert.assertEquals(14L, (long) config.getChatRetries());
- Assert.assertEquals(15, (int) config.getRecordBufferSizeConfigured());
- Assert.assertEquals(16, (int) config.getRecordBufferOfferTimeout());
- Assert.assertEquals(17, (int) config.getMaxRecordsPerPollConfigured());
- Assert.assertEquals(Duration.standardSeconds(15), config.getHttpTimeout());
- Assert.assertEquals(Duration.standardSeconds(95), config.getShutdownTimeout());
- Assert.assertEquals(Duration.standardSeconds(120), config.getRepartitionTransitionDuration());
+ Assertions.assertNull(config.getBasePersistDirectory());
+ Assertions.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
+ Assertions.assertEquals(100, config.getMaxRowsInMemory());
+ Assertions.assertEquals(100, config.getMaxRowsPerSegment().intValue());
+ Assertions.assertEquals(new Period("PT1H"), config.getIntermediatePersistPeriod());
+ Assertions.assertEquals(100, config.getMaxPendingPersists());
Review Comment:
## CodeQL / Deprecated method or constructor invocation
Invoking [SeekableStreamIndexTaskTuningConfig.getMaxPendingPersists](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/11693)
##########
extensions-contrib/rabbit-stream-indexing-service/src/test/java/org/apache/druid/indexing/rabbitstream/supervisor/RabbitStreamSupervisorTuningConfigTest.java:
##########
@@ -87,21 +82,21 @@
TuningConfig.class)),
TuningConfig.class);
- Assert.assertNull(config.getBasePersistDirectory());
- Assert.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
- Assert.assertEquals(150000, config.getMaxRowsInMemory());
- Assert.assertEquals(5_000_000, config.getMaxRowsPerSegment().intValue());
- Assert.assertEquals(new Period("PT10M"), config.getIntermediatePersistPeriod());
- Assert.assertEquals(0, config.getMaxPendingPersists());
- // Assert.assertEquals(IndexSpec.getDefault(), config.getIndexSpec());
- Assert.assertEquals(false, config.isReportParseExceptions());
- Assert.assertEquals(java.time.Duration.ofMinutes(15).toMillis(), config.getHandoffConditionTimeout());
- Assert.assertNull(config.getWorkerThreads());
- Assert.assertEquals(8L, (long) config.getChatRetries());
- Assert.assertEquals(Duration.standardSeconds(10), config.getHttpTimeout());
- Assert.assertEquals(Duration.standardSeconds(80), config.getShutdownTimeout());
- Assert.assertEquals(Duration.standardSeconds(120), config.getRepartitionTransitionDuration());
- Assert.assertEquals(100, config.getMaxRecordsPerPollOrDefault());
+ Assertions.assertNull(config.getBasePersistDirectory());
+ Assertions.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
+ Assertions.assertEquals(150000, config.getMaxRowsInMemory());
+ Assertions.assertEquals(5_000_000, config.getMaxRowsPerSegment().intValue());
+ Assertions.assertEquals(new Period("PT10M"), config.getIntermediatePersistPeriod());
+ Assertions.assertEquals(0, config.getMaxPendingPersists());
Review Comment:
## CodeQL / Deprecated method or constructor invocation
Invoking [SeekableStreamIndexTaskTuningConfig.getMaxPendingPersists](1) should be avoided because it has been deprecated.
[Show more details](https://github.com/apache/druid/security/code-scanning/11692)
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test: migrate second extensions-contrib batch to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19915:
URL: https://github.com/apache/druid/pull/19915#discussion_r3736371651
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssDataSegmentMoverTest.java:
##########
@@ -150,27 +154,29 @@
), ImmutableMap.of("bucket", "DOES NOT EXIST", "baseKey", "baseKey"));
}
- @Test(expected = SegmentLoadingException.class)
- public void testFailsToMoveMissing() throws Exception
+ @Test
+ public void testFailsToMoveMissing()
{
- MockClient client = new MockClient();
- OssDataSegmentMover mover = new OssDataSegmentMover(Suppliers.ofInstance(client), new OssStorageConfig());
- 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"));
+ assertThrows(SegmentLoadingException.class, () -> {
+ MockClient client = new MockClient();
+ OssDataSegmentMover mover = new OssDataSegmentMover(Suppliers.ofInstance(client), new OssStorageConfig());
+ 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:
Thanks for flagging this. This call predates the JUnit 5 migration; the PR only moved it into an assertThrows lambda. Migrating the deprecated DataSegment constructor to a builder is outside this PR’s scope, so I’m intentionally leaving it unchanged.
##########
extensions-contrib/rabbit-stream-indexing-service/src/test/java/org/apache/druid/indexing/rabbitstream/supervisor/RabbitStreamSupervisorTuningConfigTest.java:
##########
@@ -87,21 +82,21 @@
TuningConfig.class)),
TuningConfig.class);
- Assert.assertNull(config.getBasePersistDirectory());
- Assert.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
- Assert.assertEquals(150000, config.getMaxRowsInMemory());
- Assert.assertEquals(5_000_000, config.getMaxRowsPerSegment().intValue());
- Assert.assertEquals(new Period("PT10M"), config.getIntermediatePersistPeriod());
- Assert.assertEquals(0, config.getMaxPendingPersists());
- // Assert.assertEquals(IndexSpec.getDefault(), config.getIndexSpec());
- Assert.assertEquals(false, config.isReportParseExceptions());
- Assert.assertEquals(java.time.Duration.ofMinutes(15).toMillis(), config.getHandoffConditionTimeout());
- Assert.assertNull(config.getWorkerThreads());
- Assert.assertEquals(8L, (long) config.getChatRetries());
- Assert.assertEquals(Duration.standardSeconds(10), config.getHttpTimeout());
- Assert.assertEquals(Duration.standardSeconds(80), config.getShutdownTimeout());
- Assert.assertEquals(Duration.standardSeconds(120), config.getRepartitionTransitionDuration());
- Assert.assertEquals(100, config.getMaxRecordsPerPollOrDefault());
+ Assertions.assertNull(config.getBasePersistDirectory());
+ Assertions.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
+ Assertions.assertEquals(150000, config.getMaxRowsInMemory());
+ Assertions.assertEquals(5_000_000, config.getMaxRowsPerSegment().intValue());
+ Assertions.assertEquals(new Period("PT10M"), config.getIntermediatePersistPeriod());
+ Assertions.assertEquals(0, config.getMaxPendingPersists());
Review Comment:
Thanks for flagging this. This call predates the JUnit 5 migration; this PR only changes Assert to Assertions here. Deprecated API cleanup is outside this PR’s scope, so I’m intentionally leaving it unchanged.
##########
extensions-contrib/rabbit-stream-indexing-service/src/test/java/org/apache/druid/indexing/rabbitstream/RabbitStreamIndexTaskTuningConfigTest.java:
##########
@@ -106,18 +101,18 @@
TuningConfig.class
);
- Assert.assertNull(config.getBasePersistDirectory());
- Assert.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
- Assert.assertEquals(100, config.getMaxRowsInMemory());
- Assert.assertEquals(100, config.getMaxRowsPerSegment().intValue());
- Assert.assertEquals(new Period("PT1H"), config.getIntermediatePersistPeriod());
- Assert.assertEquals(100, config.getMaxPendingPersists());
- Assert.assertTrue(config.isReportParseExceptions());
- Assert.assertEquals(100, config.getHandoffConditionTimeout());
- Assert.assertEquals(1000, (int) config.getRecordBufferSizeConfigured());
- Assert.assertEquals(1000, config.getRecordBufferSizeOrDefault(1_000_000_000));
- Assert.assertEquals(500, config.getRecordBufferOfferTimeout());
- Assert.assertFalse(config.isResetOffsetAutomatically());
+ Assertions.assertNull(config.getBasePersistDirectory());
+ Assertions.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
+ Assertions.assertEquals(100, config.getMaxRowsInMemory());
+ Assertions.assertEquals(100, config.getMaxRowsPerSegment().intValue());
+ Assertions.assertEquals(new Period("PT1H"), config.getIntermediatePersistPeriod());
+ Assertions.assertEquals(100, config.getMaxPendingPersists());
Review Comment:
Thanks for flagging this. This call predates the JUnit 5 migration; this PR only changes Assert to Assertions here. Deprecated API cleanup is outside this PR’s scope, so I’m intentionally leaving it unchanged.
##########
extensions-contrib/rabbit-stream-indexing-service/src/test/java/org/apache/druid/indexing/rabbitstream/supervisor/RabbitStreamSupervisorTuningConfigTest.java:
##########
@@ -134,22 +129,22 @@
TuningConfig.class)),
TuningConfig.class);
- Assert.assertNull(config.getBasePersistDirectory());
- Assert.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
- Assert.assertEquals(100, config.getMaxRowsInMemory());
- Assert.assertEquals(100, config.getMaxRowsPerSegment().intValue());
- Assert.assertEquals(new Period("PT1H"), config.getIntermediatePersistPeriod());
- Assert.assertEquals(100, config.getMaxPendingPersists());
- Assert.assertEquals(true, config.isReportParseExceptions());
- Assert.assertEquals(100, config.getHandoffConditionTimeout());
- Assert.assertEquals(12, (int) config.getWorkerThreads());
- Assert.assertEquals(14L, (long) config.getChatRetries());
- Assert.assertEquals(15, (int) config.getRecordBufferSizeConfigured());
- Assert.assertEquals(16, (int) config.getRecordBufferOfferTimeout());
- Assert.assertEquals(17, (int) config.getMaxRecordsPerPollConfigured());
- Assert.assertEquals(Duration.standardSeconds(15), config.getHttpTimeout());
- Assert.assertEquals(Duration.standardSeconds(95), config.getShutdownTimeout());
- Assert.assertEquals(Duration.standardSeconds(120), config.getRepartitionTransitionDuration());
+ Assertions.assertNull(config.getBasePersistDirectory());
+ Assertions.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
+ Assertions.assertEquals(100, config.getMaxRowsInMemory());
+ Assertions.assertEquals(100, config.getMaxRowsPerSegment().intValue());
+ Assertions.assertEquals(new Period("PT1H"), config.getIntermediatePersistPeriod());
+ Assertions.assertEquals(100, config.getMaxPendingPersists());
Review Comment:
Thanks for flagging this. This call predates the JUnit 5 migration; this PR only changes Assert to Assertions here. Deprecated API cleanup is outside this PR’s scope, so I’m intentionally leaving it unchanged.
##########
extensions-contrib/rabbit-stream-indexing-service/src/test/java/org/apache/druid/indexing/rabbitstream/RabbitStreamIndexTaskTuningConfigTest.java:
##########
@@ -61,22 +56,22 @@
TuningConfig.class
);
- Assert.assertNull(config.getBasePersistDirectory());
- Assert.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
- Assert.assertEquals(150000, config.getMaxRowsInMemory());
- Assert.assertEquals(5_000_000, config.getMaxRowsPerSegment().intValue());
- Assert.assertEquals(new Period("PT10M"), config.getIntermediatePersistPeriod());
- Assert.assertEquals(0, config.getMaxPendingPersists());
- // Assert.assertEquals(IndexSpec.getDefault(), config.getIndexSpec());
- Assert.assertEquals(false, config.isReportParseExceptions());
- Assert.assertEquals(Duration.ofMinutes(15).toMillis(), config.getHandoffConditionTimeout());
-
- Assert.assertNull(config.getRecordBufferSizeConfigured());
- Assert.assertEquals(10000, config.getRecordBufferSizeOrDefault(1_000_000_000));
- Assert.assertEquals(5000, config.getRecordBufferOfferTimeout());
-
- Assert.assertFalse(config.isSkipSequenceNumberAvailabilityCheck());
- Assert.assertFalse(config.isResetOffsetAutomatically());
+ Assertions.assertNull(config.getBasePersistDirectory());
+ Assertions.assertEquals(new OnheapIncrementalIndex.Spec(), config.getAppendableIndexSpec());
+ Assertions.assertEquals(150000, config.getMaxRowsInMemory());
+ Assertions.assertEquals(5_000_000, config.getMaxRowsPerSegment().intValue());
+ Assertions.assertEquals(new Period("PT10M"), config.getIntermediatePersistPeriod());
+ Assertions.assertEquals(0, config.getMaxPendingPersists());
Review Comment:
Thanks for flagging this. This call predates the JUnit 5 migration; this PR only changes Assert to Assertions here. Deprecated API cleanup is outside this PR’s scope, so I’m intentionally leaving it unchanged.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] test: migrate second extensions-contrib batch to JUnit 5 (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #19915:
URL: https://github.com/apache/druid/pull/19915#discussion_r3736467469
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssDataSegmentPusherTest.java:
##########
@@ -91,7 +90,7 @@ private void testPushInternal(boolean useUniquePath, String matcher) throws Exce
OssDataSegmentPusher pusher = new OssDataSegmentPusher(client, config);
// Create a mock segment on disk
- File tmp = tempFolder.newFile("version.bin");
+ File tmp = new File(tempFolder, "version.bin");
Review Comment:
Classified as pre-existing/context-only. Base commit `fe4377a4` already declared `File tmp` without `final`; this PR only changes the temporary-file creation from `tempFolder.newFile(...)` to `new File(...)` for the JUnit 5 `@TempDir` migration. No new defect is introduced, so the declaration is unchanged.
##########
extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTaskLogsTest.java:
##########
@@ -424,7 +423,8 @@ private List<Grant> testPushInternal(boolean disableAcl, String ownerId, String
OssTaskLogs taskLogs = new OssTaskLogs(ossClient, config, inputDataConfig, timeSupplier);
String taskId = "index_test-datasource_2019-06-18T13:30:28.887Z";
- File logFile = tempFolder.newFile("test_log_file");
+ File logFile = new File(tempFolder, "test_log_file");
+ Assertions.assertTrue(logFile.createNewFile());
Review Comment:
Classified as pre-existing/context-only. In base commit `fe4377a4`, both declarations were already non-final (`String taskId` and `File logFile`). This PR only changes `logFile` construction from `TemporaryFolder.newFile(...)` to `new File(...)` for the JUnit 5 `@TempDir` migration, so it introduces no defect related to these declarations.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org