-
Notifications
You must be signed in to change notification settings - Fork 224
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
test: Migrated existing tests to JUnit5 API - removed JUnit4 (2/2) #479
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, left some comments for discussion.
...racle/javafx/scenebuilder/kit/editor/panel/library/manager/DialogListItemComparatorTest.java
Show resolved
Hide resolved
kit/src/test/java/com/oracle/javafx/scenebuilder/kit/fxom/FXMLPropertiesDisablerTest.java
Show resolved
Hide resolved
kit/src/test/java/com/oracle/javafx/scenebuilder/kit/fxom/FXOMDocumentTest.java
Outdated
Show resolved
Hide resolved
.../test/java/com/oracle/javafx/scenebuilder/kit/fxom/FXMLPropertiesDisablerVariationsTest.java
Show resolved
Hide resolved
kit/src/test/java/com/oracle/javafx/scenebuilder/kit/skeleton/SkeletonFileNameProposalTest.java
Show resolved
Hide resolved
...racle/javafx/scenebuilder/kit/editor/panel/library/manager/DialogListItemComparatorTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I assume all tests (after this PR) are JUnit 5 only?
Also please check the comments.
|
||
String generatedFxml = classUnderTest.getFxmlText(false); | ||
assertTrue(generatedFxml.contains("useSystemMenuBar=\"true\"")); | ||
} | ||
|
||
@Test | ||
public void that_useSystemMenuBarProperty_not_modified_on_Linux_and_Windows() throws Exception { | ||
assumeTrue("Windows or Linux", Set.of(OS.LINUX, OS.WINDOWS).contains(OS.get())); | ||
assumeTrue(Set.of(OS.LINUX, OS.WINDOWS).contains(OS.get()), "Windows or Linux"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps adding disabled on os or enabled on os would be clearer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, very good idea. Much better than this.
|
||
String generatedFxml = classUnderTest.getFxmlText(false); | ||
assertTrue(generatedFxml.contains("useSystemMenuBar=\"true\"")); | ||
} | ||
|
||
@Test | ||
public void that_useSystemMenuBarProperty_is_disabled_on_MacOS() throws Exception { | ||
assumeTrue("MacOS", OS.get() == OS.MAC); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps adding disabled on os or enabled on os would be clearer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JUnit4 will no longer work.
Contributes to effort to modernize test infrastructure.
All existing tests migrated to JUnit5 API.
This is the follow up to PR #478
Issue
Fixes #449
Progress