Copilot commented on code in PR #14:
URL: https://github.com/apache/pulsar-connectors/pull/14#discussion_r3044769585
##########
file/src/main/java/org/apache/pulsar/io/file/FileListingThread.java:
##########
@@ -97,7 +97,7 @@ public void run() {
}
try {
- sleep(pollingInterval - 1);
+ sleep(pollingInterval);
} catch (InterruptedException e) {
// Just ignore
Review Comment:
`InterruptedException` is swallowed here, so the thread cannot be shut down
cleanly and the interrupt flag is lost. Since this code is being modified,
consider restoring the interrupt status (`Thread.currentThread().interrupt()`)
and breaking out of the loop (or otherwise terminating) to make the thread
responsive to shutdown/cancellation.
```suggestion
Thread.currentThread().interrupt();
break;
```
##########
file/src/test/java/org/apache/pulsar/io/file/FileListingThreadTest.java:
##########
@@ -163,6 +163,35 @@ public final void maximumAgeTest() throws IOException {
}
}
+ @Test
+ public void pollingIntervalTest() throws IOException {
+ int pollingInterval = 100;
+ int tolerance = 20;
+
+ Map<String, Object> map = new HashMap<>();
+ map.put("inputDirectory", directory.toString());
+ map.put("pollingInterval", pollingInterval);
+
+ try {
+ listingThread = new FileListingThread(FileSourceConfig.load(map),
workQueue, inProcess, recentlyProcessed);
+ executor.execute(listingThread);
+
+ generateFiles(1);
+ Thread.sleep(pollingInterval + tolerance);
+
+ verify(workQueue, times(1)).offer(any(File.class));
+
+ generateFiles(1);
+ Thread.sleep(pollingInterval + tolerance);
+
+ verify(workQueue, times(2)).offer(any(File.class));
Review Comment:
This test is timing-sensitive and can be flaky on slower/loaded CI runners:
the listing thread might not get scheduled within `pollingInterval +
tolerance`, causing the `times(1)` / `times(2)` assertions to fail even when
behavior is correct. Consider replacing the fixed `Thread.sleep(...)` + exact
call counts with an eventual assertion (e.g., Mockito `verify(...,
timeout(...)).offer(...)` / `after(...)`, or a small polling loop with an
overall deadline) so the test waits up to a bound for the expected offers.
--
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]
