Copilot commented on code in PR #17042:
URL: https://github.com/apache/pinot/pull/17042#discussion_r2443470276
##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/util/SegmentDeletionManagerTest.java:
##########
@@ -311,7 +311,8 @@ public void testRemoveDeletedSegments()
// Try to remove empty directory in the next run
deletionManager.removeAgedDeletedSegments(leadControllerManager);
- Assert.assertFalse(dummyDir2.exists());
+ TestUtils.waitForCondition((aVoid) -> !dummyDir2.exists(), 1000, 100000,
+ "dummyDir2 still exists");
Review Comment:
The 100000 ms timeout can cause a failing test to hang for up to 100
seconds, slowing the suite. Consider reducing the timeout (e.g., 10000) or
tightening the polling interval (e.g., 200 ms) to fail faster while still
mitigating flakiness.
##########
pinot-controller/src/test/java/org/apache/pinot/controller/helix/core/util/SegmentDeletionManagerTest.java:
##########
@@ -311,7 +311,8 @@ public void testRemoveDeletedSegments()
// Try to remove empty directory in the next run
deletionManager.removeAgedDeletedSegments(leadControllerManager);
- Assert.assertFalse(dummyDir2.exists());
+ TestUtils.waitForCondition((aVoid) -> !dummyDir2.exists(), 1000, 100000,
+ "dummyDir2 still exists");
Review Comment:
The magic numbers 1000 and 100000 reduce readability; consider replacing
them with named constants (e.g., POLL_INTERVAL_MS, TIMEOUT_MS) or using
TimeUnit conversions and numeric underscores (e.g., 1_000, 100_000) for clarity.
--
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]
