[GitHub] [helix] kaisun2000 commented on pull request #1227: Implement thread leakage check, fixing major thread leakage test and some flaky test

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

kaisun2000 commented on pull request #1227:
URL: https://github.com/apache/helix/pull/1227#issuecomment-678000265


   > @kaisun2000 I strongly recommend splitting the logic changes from the pure testing fixes.
   > I mainly reviewed the logic change part. The test changes are too long to review. Could you please re-org the PR?
   
   Let us sync about it offline. There are conflict of different requirements.
   
   1/  If our goal is to have all the test pass in github, we have to run it consistently till it converge to no failing test with all the changes in. Otherwise, how can we verify? 
   2/ The huge diff would be difficult to review. Yes, that is another concern.
   3/ We have to make test converge quickly as people are checking in new code and potentially introducing new unstableness. We have to act quick, or it will not converge. 


----------------------------------------------------------------
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