RFR: 8339510: [TestBug] Convert system tests to JUnit 5 [v4]

[Kevin Rushforth]

On Thu, 19 Sep 2024 21:24:59 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> tests/system/src/test/java/test/javafx/scene/Snapshot1Test.java line 139:
>> 
>>> 137:             // Should throw IllegalStateException
>>> 138:             tmpScene.snapshot(p -> {
>>> 139:                 throw new RuntimeException("Should never get here");
>> 
>> Suggestion: Use `fail`, rather than changing the Error into a RuntimeException.
>
> won't compile with `fail()`.  any exception should be fine.

In that case, `throw new AssertionError` seems the most equivalent change, in that it preserves the behavior of throwing an `Error` rather than a `RuntimeException` -- typically an Error is better for a case of "can't get here". I'll admit that it doesn't matter in this particular test, so it's just a suggestion. Feel free to ignore it.

>> tests/system/src/test/java/test/javafx/scene/UIRenderSnapToPixelTest.java line 76:
>> 
>>> 74:                 Assertions.assertEquals(0, ((sp.snappedBottomInset() * scale) + epsilon) % 1, 0.0001, "Bottom inset not snapped to pixel");
>>> 75:                 Assertions.assertEquals(0, ((sp.snappedLeftInset() * scale) + epsilon) % 1, 0.0001, "Left inset not snapped to pixel");
>>> 76:                 Assertions.assertEquals(0, ((sp.snappedRightInset() * scale) + epsilon) % 1, 0.0001, "Right inset not snapped to pixel");
>> 
>> Minor: these lines are geting a bit long. Maybe use static imports?
>
> still too long, but I do prefer having the class qualifier over static import (though static imports make more sense in the tests).
> 
> I think tests is one area where we can relax the maximum line length rule (we already violate it in many places anyway)

Yeah, don't worry about the line length, that was just an observation.

> I do prefer having the class qualifier over static import (though static imports make more sense in the tests).

So do I in most cases. For tests, I prefer static imports of the methods in `Assertions` and `Assumptions`, which is also the pattern recommended by the JUnit docs.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1569#discussion_r1768765939
PR Review Comment: https://git.openjdk.org/jfx/pull/1569#discussion_r1768770602