[GitHub] [accumulo] ctubbsii commented on pull request #1816: Fixes #1763 - Flaky test: SimpleTimerTest

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

ctubbsii commented on pull request #1816:
URL: https://github.com/apache/accumulo/pull/1816#issuecomment-736073587


   > Not sure if I should extract the code containing the while(true) and create a separate method in order to reuse that portion of code.
   
   No. These changes aren't big enough to warrant that.
   
   > Not sure if I should add a local variable for the timeout length (even though there is one test case that requires a longer timeout).
   
   No. Your timeouts are fine.
   
   > I added a timeout to testSingleton() which probably doesn't need it so I am unsure whether I should remove it or keep it in.
   
   In general, we try to make minimal unrelated changes, to make it easier to review, but it's often a judgement call. In this case, I would leave it in. It's a tiny change, and is related to your adding a timeout to all the other test cases.


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