Re: RFR (S) 8223441: HeapMonitorStatArrayCorrectnessTest fails due to sampling determinism

Wed, 08 May 2019 02:01:53 -0700

Hi Jc,

A couple of minor comments.

  37   private static final int maxCount = 5;
  Constant names are better to start from a capital letter.
  Also, I'd suggest 10 instead of 5 to make it more reliable.
  But it depend on how much it potentially can make the test to run slower.

  94       // If we failed maxCount times, through the exception.
  A typo: replace "through" with "throw".


Otherwise, I'm Okay with this fix.
No need for another webrev.


Thanks,
Serguei
 

On 5/7/19 19:47, Jean Christophe Beyler wrote:
Hi all,

Could I get a review for this test fix:


Backstory is:
When I fixed https://bugs.openjdk.java.net/browse/JDK-8215113, I fixed this failing test in the "actually make the error calculation correct". But it turns out the test itself failed 7 out of 10 times and when we tested it, we were apparently "lucky".

This fix makes the test pass 5k times so I am hopeful it will not show up again.

Extra notes:
   - I fixed this by adding a loop that says: if we have not converged yet, try again at max 5 times; generally we don't converge if the sampling intervals chosen at the start just have pushed us far enough from the average we expect that it takes more samples to get us back on track

Thanks,
Jc

Ps: FWIW, I'm going to look at the flakyness of the rest of the suite and see if anything pops up to reduce the test bug noise of this suite

Reply via email to