test(s3stream): Unit tests for TrafficRateLimiter #2383
Conversation
TwoOnefour
requested review from
superhx,
SCNieh,
Chillax-0v0 and
Gezi-lzq
as code owners
TwoOnefour
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds unit tests for the TrafficRateLimiter class to verify its rate update and consumption behaviors.
- Introduces a test to check boundary conditions in currentRate updates.
- Adds a test to ensure that consuming traffic respects the set rate limit and timing constraints.
Comments suppressed due to low confidence (2)
s3stream/src/test/java/com/automq/stream/s3/operator/TrafficRateLimiterTest.java:56
- Consider moving the consumeStarted.countDown() call to immediately after starting the consume method (before joining on its completion) to better ensure that the update(0) call occurs while consumption is still in progress, thereby testing concurrent behavior more accurately.
consumeStarted.countDown();
s3stream/src/test/java/com/automq/stream/s3/operator/TrafficRateLimiterTest.java:68
- [nitpick] Consider revising the fixed 5-second threshold to account for potential variability in execution timing (e.g., by using a more flexible condition or providing additional context), in order to reduce the risk of test flakiness under varying system loads.
assertTrue(duration / 1000 <= 5);
8e55abe to
a8a6c70
Compare
|
@TwoOnefour Please ensure that checkstyle passes locally and submit all your code changes in one batch, rather than making incremental adjustments piece by piece. |
sry, I will handler it carefully. |
|
@Chillax-0v0 |
#2365