Hi Eugene, that looks right to me. (did that fix it for you?) In addition to the anti-pattern I mentioned earlier, another one to look for is slow running tests -- often times a test will run slowly that could be coded in a different way to run much more quickly. Notice here that we don't even wait the second if the test is already passing. Although we might run for a much longer time if needed. (this all adds up when you have hundreds/thousands of tests).
btw, you might put that comment directly into your assert. Also take a look at Assert.fail("foo") instead of assertTrue(false). (it's a nit though). Patrick On Tue, Jun 21, 2011 at 2:03 PM, Eugene Koontz <ekoo...@hiro-tan.org> wrote: > On 6/21/11 12:45 PM, Patrick Hunt wrote: >> >> Such uses of sleep are just asking for trouble. Take a look at the use >> of sleep in testSessionMove in the same class for a better way to do >> this. I had gone through all the tests a while back, replacing all the >> "sleep(x)" with something like this testSessionMove pattern (retry >> with a max limit that's very long). During reviews we should look for >> anti-patterns like this and address them before commit. >> >> Patrick > > Thanks a lot for bringing this up, Camille. I had exactly this problem > (QuorumTest.testFollowersStartAfterLeader failing) yesterday and today . > Would the attached patch be the fix in the spirit of the pattern you're > describing, Patrick? > > -Eugene > > >