[jdk8u-dev] RFR: 8305329: [8u] Unify test libraries into single test library - step 1

[Zdenek Zambersky]

On Sat, 12 Aug 2023 12:50:22 GMT, Andrew John Hughes <andrew at openjdk.org> wrote:

>> I would like to start effort to unify test libraries in openjdk 8 into one unified library. This is first step of that effort, which moves newest copy of jdk test library (added with jfr backport) from `jdk/test/lib` to `test/lib`. For more details please continue reading. 
>> 
>> **Problem:**
>> Test libraries in jdk8 are a bit messy.
>> There are currently 3 different test libraries:
>> - hotspot testlibrary - placed in [hotspot/test/testlibrary](https://github.com/openjdk/jdk8u-dev/tree/89aeae16e85ddfbd581cb86d0b0480b1e2d50e99/hotspot/test/testlibrary), (pkgs `com.oracle.java.testlibrary`, `sun.hotspot...` ), includes testlibrary tests in [hotspot/test/testlibrary_tests](https://github.com/openjdk/jdk8u-dev/tree/89aeae16e85ddfbd581cb86d0b0480b1e2d50e99/hotspot/test/testlibrary_tests)
>> - jdk testlibrary (old) - placed in [jdk/test/lib/testlibrary](https://github.com/openjdk/jdk8u-dev/tree/master/jdk/test/lib/testlibrary), (pkgs `jdk.testlibrary`, `com.oracle.testlibrary.jsr292`)
>> - jdk test lib - placed directly in [jdk/test/lib](https://github.com/openjdk/jdk8u-dev/tree/master/jdk/test/lib), was added with [jfr backport](https://github.com/openjdk/jdk8u-dev/commit/df7e09043392d5952d522a28702c6e5ec3e8492e), (pkgs `jdk.test.lib` `sun.hotspot...`)
>> 
>> Many test library classes exist in multiple different copies (using different pkgs). Few examples:
>> - Platform.java - 3 copies ([hs](https://github.com/openjdk/jdk8u-dev/blob/89aeae16e85ddfbd581cb86d0b0480b1e2d50e99/hotspot/test/testlibrary/com/oracle/java/testlibrary/Platform.java), [jdk (old)](https://github.com/openjdk/jdk8u-dev/blob/master/jdk/test/lib/testlibrary/jdk/testlibrary/Platform.java), [jdk (jfr)](https://github.com/openjdk/jdk8u-dev/blob/master/jdk/test/lib/jdk/test/lib/Platform.java))
>> - OutputAnalyzer.java - 3 copies ([hs](https://github.com/openjdk/jdk8u-dev/blob/89aeae16e85ddfbd581cb86d0b0480b1e2d50e99/hotspot/test/testlibrary/com/oracle/java/testlibrary/OutputAnalyzer.java), [jdk (old)](https://github.com/openjdk/jdk8u-dev/blob/master/jdk/test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java), [jdk (jfr)](https://github.com/openjdk/jdk8u-dev/blob/master/jdk/test/lib/jdk/test/lib/process/OutputAnalyzer.java))
>> - WhiteBox.java - 2 copies ([hotspot](https://github.com/openjdk/jdk8u-dev/blob/89aeae16e85ddfbd581cb86d0b0480b1e2d50e99/hotspot/test/testlibrary/whitebox/sun/hotspot/WhiteBox.java), [jdk (jfr)](https://github.com/openjdk/jdk8u-dev/blob/master/jdk/test/lib/sun/hotspot/WhiteBox.j...
>
> I think we should also probably have a step 4 to actually move the tests. That will then solve the backporting problem where tests keep being introduced in the wrong location.

Hi @gnu-andrew,
thank you for your review,

> This is something I'm aware of and I've thought about tackling it in a similar way to you a few times. Thanks for finding the time to do so which has eluded me so far.
> 
> I don't have any real issue with this PR, but then I guess this is the easy one. I'm a bit concerned about steps 2 & 3, because you seem to be suggesting that the existing HotSpot & JDK test libraries should be migrated to work with the ones from the JFR library. This seems like the wrong direction to me and a lot more work than migrating the JFR ones to work with the existing libraries (which is what the JFR backport should have done in the first place).
> 
I haven't looked in too much detail, on how they should be merged, but probably on case by case basis. I think, in case of some more generic classes, rather than trying to merge `hs` and `jdk` versions (and fixing java package), it could make sense to just go with `jfr` version (if it looks ok for 8). I mean more generic classes like `Asserts.java`, `OutputAnalyzer.java`, maybe `Platform.java`. In other cases like broken/risky(low-level) stuff it would probably be better to go with `hs` (or `jdk`) version. (e.g. `whitebox` stuff)

> As you say yourself, some of the JFR imported library code doesn't even compile and it includes stuff for newer JDKs which isn't appropriate in 8u. This PR would seem like a good opportunity to remove rather than move any of these files that don't compile (which presumably also means they aren't being used by the JFR tests).
> 
> Have you verified that all the moved files actually compile? And if not, can we check that and delete any that don't?

I have not excluded any files, just moved all of them. However excluding broken stuff here is probably good idea. Keeping and trying to fix unused broken code, would probably be waste of time, as they can be re-added later, if needed by future backport. (I'll take a look at that)

> 
> Then, in the HotSpot stage, we can merge in the HotSpot library and replace any duplicates with the 8u versions, and see what (if anything) breaks in the JFR tests.

For hotspot-specific or low-level stuff it is probably safest approach. In case of more generic classes (which are also used by `jdk` tests), keeping `jfr` version may be easier and safer option.

Also we could use this opportunity to move whitebox classes to `jdk.test.lib` package (used by jdk 17+, see [JDK-8067223](https://bugs.openjdk.org/browse/JDK-8067223), probably can be backported to 11)

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

PR Comment: https://git.openjdk.org/jdk8u-dev/pull/294#issuecomment-1677326906