On Thu, 8 Oct 2020 07:47:33 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:

>> Florian Kirmaier has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   JDK-8244297
>>   Fixed unit-test
>
> modules/javafx.base/src/test/java/test/util/memory/JMemoryBuddy.java line 79:
> 
>> 77:             AssertCollectable assertCollectable = new 
>> AssertCollectable(weakReference);
>> 78:             createHeapDump();
>> 79:             throw new AssertionError("Content of WeakReference was not 
>> collected. content: " + weakReference.get());
> 
> Can be replaced with `assertNull`

It's more like the opposite. assertNotNull.
But the checks are also more complicated than the standard check because it's 
combined with some GC-magic.

> modules/javafx.base/src/test/java/test/util/memory/JMemoryBuddy.java line 84:
> 
>> 82:
>> 83:     /**
>> 84:      * Checks whether the content of the WeakReference can be collected.
> 
> -> Checks whether the content of the WeakReference **is** collected.

That's not true. At the moment of method-call, the variable might not be 
collected.
It doesen't check whether it's collected, it checks whether it is "collectable".

> modules/javafx.base/src/test/java/test/util/memory/JMemoryBuddy.java line 88:
> 
>> 86:      * @return Returns true, when the provided WeakReference can be 
>> collected.
>> 87:      */
>> 88:     public static boolean checkCollectable(WeakReference weakReference) {
> 
> I would prefer a name like `checkIfCollected` or `checkIfGCed`

That's not true. see one of the other comments.

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

PR: https://git.openjdk.java.net/jfx/pull/204

Reply via email to