Copilot commented on code in PR #19923:
URL: https://github.com/apache/druid/pull/19923#discussion_r3737759470
##########
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateManagerTest.java:
##########
@@ -373,9 +373,9 @@ public void testExceptionEventSerde() throws IOException
String serialized = defaultMapper.writeValueAsString(event);
Review Comment:
`testExceptionEventSerde` currently builds a plain
`SupervisorStateManager.ExceptionEvent`, which does not serialize a
`streamException` field, and the assertion uses `Boolean.getBoolean(...)`
(system-property lookup) so the test can pass even if the JSON is wrong. This
should serialize a `SeekableStreamExceptionEvent` and assert the actual boolean
value from JSON.
##########
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/StreamChunkReaderTest.java:
##########
@@ -83,6 +76,12 @@ public class StreamChunkReaderTest
@Mock
private SettableByteEntityReader mockedByteEntityReader;
+ @BeforeEach
+ public void setup()
+ {
+ MockitoAnnotations.openMocks(this);
+ }
Review Comment:
`MockitoAnnotations.openMocks(this)` returns an `AutoCloseable` that should
be closed after each test to avoid leaking Mockito resources. Also,
`FileUtils.createTempDir()` does not auto-delete, so the per-test temporary
directory should be deleted in an `@AfterEach` cleanup to preserve the old
`TemporaryFolder` rule semantics.
##########
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunnerTest.java:
##########
@@ -116,9 +111,10 @@ public class SeekableStreamIndexTaskRunnerTest
private StubServiceEmitter emitter;
- @Before
+ @BeforeEach
public void setup()
{
+ MockitoAnnotations.openMocks(this);
emitter = new StubServiceEmitter();
Review Comment:
`MockitoAnnotations.openMocks(this)` returns an `AutoCloseable` and should
be closed after each test to avoid leaking Mockito resources. Additionally,
`FileUtils.createTempDir()` does not auto-delete, so the per-test temp
directory should be removed in teardown to avoid accumulating temp dirs during
the test suite run.
##########
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/RecordSupplierInputSourceTest.java:
##########
@@ -69,8 +69,7 @@ public class RecordSupplierInputSourceTest extends
InitializedNullHandlingTest
private static final int NUM_ROWS = 128;
private static final String TIMESTAMP_STRING = "2019-01-01";
- @Rule
- public TemporaryFolder temporaryFolder = new TemporaryFolder();
+ private final File temporaryFolder = FileUtils.createTempDir();
@Test
Review Comment:
`FileUtils.createTempDir()` does not auto-delete, so this per-test temporary
directory should be cleaned up in an `@AfterEach` teardown (the old JUnit4
`TemporaryFolder` rule did this automatically).
##########
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SequenceMetadataTest.java:
##########
@@ -64,6 +63,12 @@ public class SequenceMetadataTest
@Mock
private TaskToolbox mockTaskToolbox;
+ @BeforeEach
+ public void setup()
+ {
+ MockitoAnnotations.openMocks(this);
+ }
Review Comment:
`MockitoAnnotations.openMocks(this)` returns an `AutoCloseable` that should
be closed in an `@AfterEach` to avoid leaking Mockito resources across the test
suite.
##########
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskTestBase.java:
##########
@@ -218,18 +216,20 @@ public SeekableStreamIndexTaskTestBase(
this.lockGranularity = lockGranularity;
}
- @Before
+ @BeforeEach
public void setupBase()
{
+ derby.before();
emitter = new StubServiceEmitter();
emitter.start();
EmittingLogger.registerEmitter(emitter);
}
- @After
+ @AfterEach
public void tearDownBase()
{
emitter.close();
+ derby.after();
}
Review Comment:
`tempFolder` is now created via `FileUtils.createTempDir()`, which does not
auto-delete. Since the old JUnit4 `TemporaryFolder` rule cleaned up after each
test, the base teardown should delete `tempFolder` to avoid accumulating large
temp directories across the seekable-stream test suite.
--
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]
