8340464: [TestBug] Convert parametrized base tests to JUnit 5 #1759

jaybhaskar · 2025-04-03T12:34:37Z

Migrated JUnit 4 tests in the jafax.base module to JUnit 5, replacing deprecated APIs, updating assertions, and refactoring test structures to align with JUnit 5's improved features.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

JDK-8340464: [TestBug] Convert parametrized base tests to JUnit 5 (Enhancement - P4)

Reviewers

Andy Goryachev (@andy-goryachev-oracle - Reviewer)
Ambarish Rapte (@arapte - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1759/head:pull/1759
$ git checkout pull/1759

Update a local copy of the PR:
$ git checkout pull/1759
$ git pull https://git.openjdk.org/jfx.git pull/1759/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1759

View PR using the GUI difftool:
$ git pr show -t 1759

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1759.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-04-03T12:35:00Z

👋 Welcome back jbhaskar! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-04-03T12:36:00Z

@jaybhaskar This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8340464: [TestBug] Convert parametrized base tests to JUnit 5

Reviewed-by: angorya, arapte

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 6 new commits pushed to the master branch:

1b26b66: 8351878: RichTextArea: copy/paste issues
f31d00d: 8328716: [TestBug] Screen capturing utility for failed tests
a52e2fa: 8352746: [TestBug] Monkey Tester Application Update 5
... and 3 more: https://git.openjdk.org/jfx/compare/62f94bb0be7f1618271672f417225e12b4d30ff8...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

mlbridge · 2025-04-03T13:15:31Z

Webrevs

andy-goryachev-oracle

First batch of comments (up until ObservableArrayTest) - the general direction is good, but I would suggest to re-work the set up in all the parameterized tests, mainly for simplicity and consistency.

modules/javafx.base/src/test/java/test/com/sun/javafx/binding/BidirectionalBindingTest.java

...x.base/src/test/java/test/com/sun/javafx/binding/BidirectionalBindingWithConversionTest.java

modules/javafx.base/src/test/java/test/com/sun/javafx/event/EventDispatchChainTest.java

...s/javafx.base/src/test/java/test/javafx/beans/property/adapter/JavaBeanPropertyTestBase.java

modules/javafx.base/src/test/java/test/javafx/binding/BindingsCreateBindingTest.java

modules/javafx.base/src/test/java/test/javafx/binding/BindingsEqualsTest.java

modules/javafx.base/src/test/java/test/javafx/binding/BindingsNumberCalculationsTest.java

modules/javafx.base/src/test/java/test/javafx/binding/BindingsNumberCastTest.java

modules/javafx.base/src/test/java/test/javafx/binding/GenericBindingTest.java

arapte

Providing a few minor comments.

...s/javafx.base/src/test/java/test/com/sun/javafx/property/adapter/PropertyDescriptorTest.java

....base/src/test/java/test/com/sun/javafx/property/adapter/ReadOnlyPropertyDescriptorTest.java

...s/javafx.base/src/test/java/test/javafx/util/converter/LocalDateTimeStringConverterTest.java

modules/javafx.base/src/test/java/test/com/sun/javafx/binding/BidirectionalBindingTest.java

andy-goryachev-oracle

Found some issues, please take a look.

modules/javafx.base/src/test/java/test/javafx/binding/BindingsNumberCastTest.java

modules/javafx.base/src/test/java/test/javafx/collections/ObservableListEmptyTest.java

modules/javafx.base/src/test/java/test/javafx/collections/ObservableListTest.java

modules/javafx.base/src/test/java/test/javafx/util/converter/DateStringConverterTest.java

modules/javafx.base/src/test/java/test/javafx/util/converter/DateTimeStringConverterTest.java

modules/javafx.base/src/test/java/test/javafx/util/converter/ParameterizedConverterTest.java

modules/javafx.base/src/test/java/test/javafx/util/converter/TimeStringConverterTest.java

jaybhaskar

Reviewed , thanks for the support

arapte

Here are few more junit4 imports that should be removed too.
Please take a look.

modules/javafx.base/src/test/java/test//javafx/binding/When_Boolean_Test.java:77: org.junit.Assert.assertEquals(expected, binding.getValue());
modules/javafx.base/src/test/java/test//javafx/binding/When_Object_Test.java:78: org.junit.Assert.assertEquals(expected, binding.getValue());
modules/javafx.base/src/test/java/test//javafx/binding/When_String_Test.java:77: org.junit.Assert.assertEquals(expected, binding.getValue());
modules/javafx.base/src/test/java/test//javafx/beans/value/ObservableValueSubscriptionsTest.java:28:import static org.junit.Assert.assertNull;
modules/javafx.base/src/test/java/test//javafx/collections/ObservableSubListIteratorTest.java:37:import static org.junit.Assert.assertEquals;

modules/javafx.base/src/test/java/test/javafx/util/converter/DateStringConverterTest.java

modules/javafx.base/src/test/java/test/javafx/util/converter/DateTimeStringConverterTest.java

andy-goryachev-oracle · 2025-04-07T16:41:31Z

/reviewers 2

openjdk · 2025-04-07T16:41:50Z

@andy-goryachev-oracle
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

andy-goryachev-oracle

looks good now, with two very minor suggestions (will re-approve if you decide to update). thanks!

modules/javafx.base/src/test/java/test/javafx/util/converter/DateStringConverterTest.java

modules/javafx.base/src/test/java/test/javafx/util/converter/DateTimeStringConverterTest.java

andy-goryachev-oracle

lgtm

arapte

LGTM

andy-goryachev-oracle · 2025-04-08T14:43:30Z

The GHA failure is unrelated, see JDK-8353947
GHA: Windows build now fails with connection error

jaybhaskar · 2025-04-08T14:44:49Z

/integrate

andy-goryachev-oracle · 2025-04-08T14:46:02Z

mock push for github gradle error

you could simply re-run the GHA jobs from the UI: click on the "..." [more actions] link, click on the failed job, there is a button somewhere at the top right with "Re-run failed jobs".

openjdk · 2025-04-08T14:46:38Z

Going to push as commit 61a248d.
Since your change was applied there have been 6 commits pushed to the master branch:

1b26b66: 8351878: RichTextArea: copy/paste issues
f31d00d: 8328716: [TestBug] Screen capturing utility for failed tests
a52e2fa: 8352746: [TestBug] Monkey Tester Application Update 5
... and 3 more: https://git.openjdk.org/jfx/compare/62f94bb0be7f1618271672f417225e12b4d30ff8...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-04-08T14:46:53Z

@jaybhaskar Pushed as commit 61a248d.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8340464: [TestBug] Convert parametrized base tests to JUnit 5

b4f95bd

remove trailing white space

da45377

openjdk bot added the rfr Ready for review label Apr 3, 2025

andy-goryachev-oracle reviewed Apr 3, 2025

View reviewed changes

arapte reviewed Apr 4, 2025

View reviewed changes

simplify test setup according to review

cf46ee0

openjdk bot removed the rfr Ready for review label Apr 4, 2025

missing review change and remove trailing white space

5f79e45

openjdk bot added the rfr Ready for review label Apr 4, 2025

jaybhaskar added 2 commits April 4, 2025 09:48

Merge remote-tracking branch 'upstream/master' into juni5bastest

03e9dd5

re-order imports

7668601

andy-goryachev-oracle suggested changes Apr 4, 2025

View reviewed changes

change according to review

89e2347

openjdk bot removed the rfr Ready for review label Apr 5, 2025

remove whitespace

1a5cdf5

openjdk bot added the rfr Ready for review label Apr 5, 2025

jaybhaskar commented Apr 5, 2025

View reviewed changes

jaybhaskar requested review from andy-goryachev-oracle and arapte April 6, 2025 09:30

arapte suggested changes Apr 7, 2025

View reviewed changes

modules/javafx.base/src/test/java/test/javafx/util/converter/DateStringConverterTest.java Outdated Show resolved Hide resolved

modules/javafx.base/src/test/java/test/javafx/util/converter/DateTimeStringConverterTest.java Outdated Show resolved Hide resolved

jaybhaskar added 2 commits April 7, 2025 16:16

further chnages for review

a0fc590

reorder imports

ac16611

andy-goryachev-oracle approved these changes Apr 7, 2025

View reviewed changes

modules/javafx.base/src/test/java/test/javafx/util/converter/DateStringConverterTest.java Outdated Show resolved Hide resolved

modules/javafx.base/src/test/java/test/javafx/util/converter/DateTimeStringConverterTest.java Outdated Show resolved Hide resolved

make public data to private

a1c5e2d

jaybhaskar requested a review from andy-goryachev-oracle April 7, 2025 17:20

jaybhaskar requested a review from arapte April 7, 2025 17:20

andy-goryachev-oracle approved these changes Apr 7, 2025

View reviewed changes

mock push for github gradle error

513f053

arapte approved these changes Apr 8, 2025

View reviewed changes

remove extra line

6a340ec

arapte approved these changes Apr 8, 2025

View reviewed changes

andy-goryachev-oracle approved these changes Apr 8, 2025

View reviewed changes

openjdk bot added the ready Ready to be integrated label Apr 8, 2025

openjdk bot added the integrated Pull request has been integrated label Apr 8, 2025

openjdk bot closed this Apr 8, 2025

openjdk bot removed ready Ready to be integrated rfr Ready for review labels Apr 8, 2025

jaybhaskar deleted the juni5bastest branch April 8, 2025 14:47

nlisker mentioned this pull request Aug 24, 2025

8250802: Refactor StringConverter and its subclasses #1880

Open

4 tasks

8340464: [TestBug] Convert parametrized base tests to JUnit 5 #1759

8340464: [TestBug] Convert parametrized base tests to JUnit 5 #1759

Uh oh!

Conversation

jaybhaskar commented Apr 3, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Apr 3, 2025

Uh oh!

openjdk bot commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arapte left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaybhaskar left a comment

Choose a reason for hiding this comment

Uh oh!

arapte left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andy-goryachev-oracle commented Apr 7, 2025

Uh oh!

openjdk bot commented Apr 7, 2025

Uh oh!

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

Uh oh!

arapte left a comment

Choose a reason for hiding this comment

Uh oh!

andy-goryachev-oracle commented Apr 8, 2025

jaybhaskar commented Apr 3, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Apr 3, 2025 •

edited

Loading

mlbridge bot commented Apr 3, 2025 •

edited

Loading