On Sun, 3 May 2020 21:39:41 GMT, Florian Kirmaier <fkirma...@openjdk.org> wrote:
> It's based on the discussion of my previous PR: > https://github.com/openjdk/jfx/pull/71 > > I Added test utility class copied from JMemoryBuddy and used it to simplify 4 > of the existing unit tests. > > It's a direct copy of my project > [JMemoryBuddy](https://github.com/Sandec/JMemoryBuddy) without any changes. > I'm also using it in most of the projects I'm involved with and in my > experience, the tests with this Library are very > stable. I can't remember wrong test results. Sometimes the memory behaviour > of some libraries itself is not stable but > the tests with JMemoryBuddy are basically always correct. I'll put this on my queue to review and test it. I left some high-level comments below. modules/javafx.base/src/test/java/de/sandec/jmemorybuddy/JMemoryBuddy.java line 1: > 1: package de.sandec.jmemorybuddy; > 2: This file needs a standard Oracle copyright header, which you can copy from any of the other test files. Also, this utility class should be moved into a package matching the pattern we already use for our tests. Something like `test.util.memory`? modules/javafx.base/src/test/java/de/sandec/jmemorybuddy/JMemoryBuddy.java line 145: > 144: > 145: public static void createHeapDump() { > 146: try { I don't think we want automatic creation of heap dumps by our unit test suite. That would need a larger discussion. I recommend to either comment this out, or else to qualify it with a system property that could be passed in via a `gradle` property (e.g., as is done for `UNSTABLE_TEST`), with a default of `false`. modules/javafx.base/src/test/java/de/sandec/jmemorybuddy/JMemoryBuddy.java line 15: > 14: > 15: public class JMemoryBuddy { > 16: Since this is a utility intended to be used by various tests, it would be very helpful to add some documentation about how to use it. ------------- PR: https://git.openjdk.java.net/jfx/pull/204
