Hi Alan, On Monday 07 October 2013 14:24:40 you wrote: > On 04/10/2013 16:10, Rob McKenna wrote: > > Hi Pavel, > > > > Thanks for sorting this out. I'm not a reviewer but hopefully Alan > > will have a look when he gets a chance. Based on the bug description > > this looks good to me though. > > > > -Rob > > I looked over the weekend and it's mostly okay (thanks Pavel for taking > one, we don't do enough execution of these tests with fastdebug builds > so I'm sure this isn't the only issue that we have). Thanks for looking on this > A minor comment is that it might be a bit cleaner to throw > RuntimeException rather than Error but it's not a big deal in this test. Error throwing on InterruptedException was added to not break code style in the code that throws only Errors all over the test. If you want to I can change this Error (and other throws too) to RuntimeException.
> The only real comment/question is whether performB should fail if > process.waitFor is interrupted, this shouldn't happen. Printing "B was interrupted while waiting for C" on InterruptedException could help if we had a regression and performC were looped. When this happens Jtreg hits timeout and kills/ends all processes. Messages of each processes that waited for another one will be printed to stderr. I think it will ease failure analysis in some situations like hanging, host or VM slowness. Thanks, Pavel
