Anyone to review this? Alan, do you have any updates on this?
On Mon, 07 of Oct 2013 20:47:22 Pavel Punegov wrote: > 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 -- Thanks, Pavel Punegov