junkaixue commented on code in PR #2587:
URL: https://github.com/apache/helix/pull/2587#discussion_r1306196330
##########
helix-core/src/test/java/org/apache/helix/integration/messaging/TestP2PNoDuplicatedMessage.java:
##########
@@ -159,25 +159,38 @@ public void testP2PStateTransitionDisabled() {
}
@Test (dependsOnMethods = {"testP2PStateTransitionDisabled"})
- public void testP2PStateTransitionEnabled() {
+ public void testP2PStateTransitionEnabled() throws Exception {
enableP2PInCluster(CLUSTER_NAME, _configAccessor, true);
long startTime = System.currentTimeMillis();
MockHelixTaskExecutor.resetStats();
// rolling upgrade the cluster
for (String ins : _instances) {
_gSetupTool.getClusterManagementTool().enableInstance(CLUSTER_NAME, ins,
false);
Assert.assertTrue(_clusterVerifier.verifyByPolling());
- verifyP2PEnabled(startTime);
+ // Since new host that receives the p2p relay message will record the
source host (controller)
+ // of the relay message as the trigger host, the top state transition
triggered by the
+ // controller should be equal to the total number of top state
transitions.
+ Assert.assertTrue(TestHelper.verify(() -> {
+ total = 0;
+ p2pTriggered = 0;
+ verifyP2PEnabled(startTime);
+ return total == p2pTriggered;
+ }, TestHelper.WAIT_DURATION),
+ "Number of successful p2p transitions when disable instance " + ins
+ ": " + p2pTriggered
+ + " , expect: " + total);
_gSetupTool.getClusterManagementTool().enableInstance(CLUSTER_NAME, ins,
true);
Assert.assertTrue(_clusterVerifier.verifyByPolling());
- verifyP2PEnabled(startTime);
+ Assert.assertTrue(TestHelper.verify(() -> {
+ total = 0;
+ p2pTriggered = 0;
+ verifyP2PEnabled(startTime);
+ return total == p2pTriggered;
+ }, TestHelper.WAIT_DURATION),
+ "Number of successful p2p transitions when enable instance " + ins +
":" + p2pTriggered
+ + " , expect:" + total);
Review Comment:
Thanks for fixing this!
These are redundant code. Can we abstract into a method and call it? Maybe
just include those into verifyP2PEnabled function or something.
--
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]
