RFR: 8338468: [TestBug] Convert controls tests to JUnit 5 [v2]

[Marius Hanl]

On Mon, 16 Sep 2024 15:23:48 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/behavior/AccordionBehaviorTest.java line 39:
>> 
>>> 37: 
>>> 38:     @Test
>>> 39:     public void focusGainedIsCaughtByBehavior() {
>> 
>> Wait, is this an empty test? Why?
>
> Intentionally did not want to change the number of tests, which should be double checked during the review.  Leaving as is for now.

Agree.

>> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinCreationTest.java line 98:
>> 
>>> 96:     }
>>> 97: 
>>> 98:     public record Parameter(
>> 
>> Minor: Can also imagine a name like `LabelParameter` or `LabelConfig`
>
> rather, this record should be declared private

Also that is a good idea.

>> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/QueryAccessibleAttributeTest.java line 50:
>> 
>>> 48:     // @BeforeEach
>>> 49:     // junit5 does not support parameterized class-level tests yet
>>> 50:     public void setup(Class<Node> nodeClass) {
>> 
>> Minor: Since `Control` is used above, should also be used here and below as Generic.
>
> hmm, the code is technically correct since Control extends Node.

yes, just a minor nitpick since relaxing the bounds is not really needed

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1561#discussion_r1764150535
PR Review Comment: https://git.openjdk.org/jfx/pull/1561#discussion_r1764149338
PR Review Comment: https://git.openjdk.org/jfx/pull/1561#discussion_r1764146934