On Tue, 8 Sep 2026 11:45:39 GMT, Marius Hanl <[email protected]> wrote:
>> Kevin Rushforth has updated the pull request incrementally with one >> additional commit since the last revision: >> >> assert that timer is/is not running as expected > > tests/system/src/test/java/test/com/sun/glass/ui/WinTimerTest.java line 42: > >> 40: import static org.junit.jupiter.api.Assertions.*; >> 41: import static org.junit.jupiter.api.Assumptions.assumeTrue; >> 42: > > Since this test should only run on Windows, I would suggest to just use the > JUnit annotation on the class level. Then you can remove the > `assumeTrue(PlatformUtil.isWindows());` call below. > > Suggestion: > > import org.junit.jupiter.api.condition.EnabledOnOs; > > @EnabledOnOs(OS.WINDOWS) That's an interesting pattern when we know that the only assumption we want to use is a platform check for a test class that (like this one) is platform-specific. I think I'll leave it as is for now, although this would be something to consider for the future. > tests/system/src/test/java/test/com/sun/glass/ui/WinTimerTest.java line 107: > >> 105: /* >> 106: * @test >> 107: * @bug 8389783 > > Minor: Needed? Or should we rather just add a `See also: JDK-XXX` or like > this: > > https://github.com/openjdk/jfx/blob/e77ca96ebc30a05356082cd56e19bcadccf7dcae/modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java#L1116 This is a jtreg-style comment indicating what bug the test is testing (not a javadoc tag). We use it in a few tests, but not consistently. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2290#discussion_r3962953653 PR Review Comment: https://git.openjdk.org/jfx/pull/2290#discussion_r3962958432
