On Tue, 6 Feb 2024 00:06:58 GMT, Laurent Bourgès <lbour...@openjdk.org> wrote:
>> Fixed scale=0 in DMarlinPrismUtils + added new test Scale0Test.java
>
> Laurent Bourgès has updated the pull request incrementally with one
> additional commit since the last revision:
>
> improved rendez-vous to collect stderr
Provided two minor comments. Otherwise ready to approve.
tests/system/src/test/java/test/com/sun/marlin/ScaleX0Test.java line 109:
> 107: Stage stage = new Stage();
> 108: stage.setScene(scene);
> 109: stage.addEventHandler(WindowEvent.WINDOW_SHOWN, e ->
> Platform.runLater(launchLatch::countDown));
Not mandatory, but can be changed to `stage.setOnShown(e ->
Platform.runLater(launchLatch::countDown));`
The import `import javafx.stage.WindowEvent;` can be removed with this change.
tests/system/src/test/java/test/com/sun/marlin/ScaleX0Test.java line 116:
> 114: if (!launchLatch.await(TIMEOUT, TimeUnit.MILLISECONDS)) {
> 115: Assert.fail("Timeout waiting for stage to show");
> 116: }
The Util class contains helper methods to bring consistency in tests that use
such common calls,
So would recommend to use: `Util.waitForLatch(launchLatch, TIMEOUT, "Failed to
show the stage");`
The error message gets prefixed with `Timeout:` in `Util.waitForLatch()`
the import `import java.util.concurrent.TimeUnit;` can be removed with this
change
-------------
PR Review: https://git.openjdk.org/jfx/pull/1348#pullrequestreview-1864882224
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1479602847
PR Review Comment: https://git.openjdk.org/jfx/pull/1348#discussion_r1479621791
