NealSun96 commented on a change in pull request #1497:
URL: https://github.com/apache/helix/pull/1497#discussion_r514524453
##########
File path:
helix-core/src/test/java/org/apache/helix/integration/spectator/TestRoutingTableProviderPeriodicRefresh.java
##########
@@ -203,34 +208,32 @@ synchronized int getRefreshCount() {
@Test
public void testPeriodicRefresh() throws InterruptedException {
- // Wait so that initial refreshes finish (not triggered by periodic
refresh timer)
- Thread.sleep(1000L);
+ // Wait to ensure that the initial refreshes finish (not triggered by
periodic refresh timer)
+ Thread.sleep(REFRESH_PERIOD_MS * 2);
// Test short refresh
int prevRefreshCount = _routingTableProvider.getRefreshCount();
- // Wait for one timer duration
- Thread.sleep(1000L);
- // The timer should have gone off, incrementing the refresh count
- Assert.assertEquals(_routingTableProvider.getRefreshCount(),
prevRefreshCount + 1);
+ // Wait for more than one timer duration
+ Thread.sleep((long) (REFRESH_PERIOD_MS * 1.5));
+ // The timer should have gone off, incrementing the refresh count by one
or two depends on the
+ // timing.
+ int newRefreshCount = _routingTableProvider.getRefreshCount();
+ Assert.assertTrue(
+ newRefreshCount == prevRefreshCount + 1 || newRefreshCount ==
prevRefreshCount + 2);
Review comment:
Talked offline: since periodic refresh can happen slightly less,
equally, or slightly more frequent than the configured wait period, I agree
with @jiajunwang 's change to use both `sleep(1500)` and `assert(+1 || +2)`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
