RFR: 8339512: [TestBug] Convert graphics tests to JUnit 5

Lukasz Kostyra lkostyra at openjdk.org
Fri Sep 20 08:04:50 UTC 2024


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

> should we set a minimum of two reviewers?

I agree, there should be another pair of eyes looking at this. Even though it's quite repetitive, it is quite a big change,

> modules/javafx.graphics/src/test/java/test/javafx/concurrent/ServiceTestBase.java line 65:
> 
>> 63:     // NOTE: This should be reverted once parametrized class tests are added to JUnit5
>> 64:     //       For now, tests call this manually
>> 65:     // @BeforeEach
> 
> I wonder if we should log a JBS ticket to fix parameterized tests by reverting `@BeforeEach`, and then simply refer to it?
> This way, once junit supports that, we can easily grep the code.
> 
> (I've used a different comment for same purpose)
> 
> What do you think?

As mentioned in another comment, it's unlikely we'll spend more time on this later on. I will remove all the NOTE comments and commented out imports and such.

> modules/javafx.graphics/src/test/java/test/javafx/stage/WindowTest.java line 222:
> 
>> 220: 
>> 221:     @Test
>> 222:     public void testSizeToSceneBeforeEachShowing() {
> 
> this looks like unrelated change:
> - why was the name changed?
> - why change this method and not testSizeToSceneAfterShowing?

Oops, it is unintentional yeah.

Most of this patch was done with multi-caret editing to tackle multiple spots affected in the same way:
- Mark affected spot
- Ask my IDE to find and put a caret on all other occurrences of selected text
- Edit them all at once

Being in the rhythm of changing imports and `@Before` to `@BeforeEach` I probably unintentionally selected and changed this occurrence of the word "Before". I'll revert this.

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

PR Comment: https://git.openjdk.org/jfx/pull/1566#issuecomment-2363084472
PR Review Comment: https://git.openjdk.org/jfx/pull/1566#discussion_r1768167064
PR Review Comment: https://git.openjdk.org/jfx/pull/1566#discussion_r1768164494


More information about the openjfx-dev mailing list