On Mon, 15 Jun 2020 09:08:14 GMT, Johan Vos <j...@openjdk.org> wrote:
>> This addresses https://bugs.openjdk.java.net/browse/JDK-8246348 > > Johan Vos has updated the pull request incrementally with one additional > commit since the last revision: > > Add test for testing Pango behavior with Character(0) and surrogate pairs The new test look good. I confirm that it fails without your patch and passes with your patch. I added a few suggestions to bring it in line with current best practices. tests/system/src/test/java/test/com/sun/javafx/font/freetype/PangoTest.java line 77: > 76: stage.show(); > 77: launchLatch.countDown(); > 78: } Our newer system tests use a `WINDOW_SHOWN` event to trigger the countdown of the latch, like this (which would be added before the stage is shown). stage.addEventHandler(WindowEvent.WINDOW_SHOWN, e -> Platform.runLater(launchLatch::countDown)); This ensures that the Stage has been shown before the first test is run. tests/system/src/test/java/test/com/sun/javafx/font/freetype/PangoTest.java line 117: > 116: if (!rDone.await(TIMEOUT, TimeUnit.MILLISECONDS)) { > 117: throw new AssertionFailedError("Timeout waiting for > runLater"); > 118: } This could also be replaced with `assertTrue("Timeout ...", rDone.await(TIMEOUT, TimeUnit.MILLISECONDS)` if you add `throws Exception` to this method and the calling test methods. modules/javafx.graphics/src/test/java/test/com/sun/javafx/text/TextLayoutTest.java line 102: > 101: @Ignore() // ignored since StubFontLoader used in tests return fonts > with null resources > 102: @Test public void utf16chars() { > 103: GlyphLayout layout = GlyphLayout.getInstance(); Since this is `@Ignore`d with no real way to make it a useful test, maybe it would be best to revert the changes to this file? ------------- PR: https://git.openjdk.java.net/jfx/pull/249
