[GitHub] [helix] NealSun96 commented on pull request #1708: Fix flaky TestAssignableInstanceManager

[GitBox <gi...@apache.org>]

NealSun96 commented on pull request #1708:
URL: https://github.com/apache/helix/pull/1708#issuecomment-825202895


   @jiajunwang I understand where you're coming from; we don't want to abuse "dependsOnMethod" for simple ordering. 
   
   However, in this case, `testUpdateAssignableInstances` is testing an "update" function which by design removes old live instance. I think the dependency is justified here. If I'm writing the tests I will combine the 3 test cases to 1 method, to be honest, because the "update" function should be tested strictly after calling the getters, to show that there is an actual update. 
   
   So I think "dependsOnMethod" is a better option here then recovering the old live instances; if you feel strongly against "dependsOnMethod", then I will combine the test cases because that's how I'd write them to begin with. 


-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org