On Fri, 1 Sep 2023 03:16:11 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> 
wrote:

> Added automated test for 8262518:SwingNode.setContent does not close previous 
> content, resulting in memory leak

tests/system/src/test/java/test/javafx/embed/swing/SwingNodeContentMemoryLeakTest.java
 line 94:

> 92:                 //Lets throw in a little sleep so we can read the output
> 93:                 try {
> 94:                     Thread.sleep(100);

I would suggest to use random delay here (random.nextInt(100)).
But make sure to set a random seed at the beginning and print it so it can be 
reproduced.  Although in this test, the outcome depends on many things that the 
test has no control over.
What do you think?

tests/system/src/test/java/test/javafx/embed/swing/SwingNodeContentMemoryLeakTest.java
 line 108:

> 106:                                                      ref.get() != 
> null).count();
> 107:                 // Sometimes panel count can shoot upto more than 3 once 
> or twice
> 108:                 // due to gc not being guranteed so this check prevents 
> false failure

guaranteed

tests/system/src/test/java/test/javafx/embed/swing/SwingNodeContentMemoryLeakTest.java
 line 116:

> 114:                 System.out.println("iteration " + count + " Panels in 
> memory: "
> 115:                                                + panelCount + " fail " + 
> fail);
> 116:                 assertFalse(fail > 2);

I actually seen the  number to shoot up to 38 (albeit with Thread.sleep(1)).  I 
wonder if there is a better way to detect failure?  May be the fact that the 
'panelCount` is ever increasing is the sign of failure, whereas if at least one 
time it's dropping we have succeeded.
Or perhaps look at the count when the test complete and fail if panelCount < 
(attempts / 2) or something

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1228#discussion_r1313115716
PR Review Comment: https://git.openjdk.org/jfx/pull/1228#discussion_r1313099511
PR Review Comment: https://git.openjdk.org/jfx/pull/1228#discussion_r1313112869

Reply via email to