On Tue, 17 Dec 2019 21:43:32 GMT, Thiago Milczarek Sayao <tsa...@openjdk.org> 
wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8232811
>> 
>> This one was hard to tackle.
>> 
>> ./gradlew -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:test --tests 
>> test.robot.javafx.scene.dialog.DialogWithOwnerSizingTest
> 
> The pull request has been updated with 1 additional commit.

The fix looks good (with a minor formatting comment). The test needs a couple 
of changes.

modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1053:

> 1052:         return;
> 1053: 
> 1054:     }

Minor: This blank line is not related to the change; probably best to remove it.

modules/javafx.graphics/src/main/native-glass/gtk/glass_window.cpp line 1289:

> 1288:         }
> 1289: 
> 1290:         gtk_window_resize(GTK_WINDOW(gtk_widget), newWidth, newHeight);

Ditto.

tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java
 line 63:

> 62:     @Test(timeout = 15000)
> 63:     public void dialogWithOwnerSizingTest() throws Exception {
> 64:         Thread.sleep(500);

This test fails on Windows 10 with 125% screen scaling. The size of the two 
dialogs are not exactly the same: 181.6 vs 180.0. I recommend either excluding 
the test or else adding a 2 pixel tolerance if the DPI scaling factor is 
non-integral.

tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java
 line 68:

> 67:         Assert.assertEquals(dialog.getDialogPane().getWidth(), 
> dialog2.getDialogPane().getWidth(), 0.0);
> 68:         Assert.assertEquals(dialog.getDialogPane().getHeight(), 
> dialog2.getDialogPane().getHeight(), 0.0);
> 69:         hide();

The expected and actual values are backwards. When it fails, it's the first 
dialog that is wrong, so the expected values should come from dialog2.

tests/system/src/test/java/test/robot/javafx/scene/dialog/DialogWithOwnerSizingTest.java
 line 69:

> 68:         Assert.assertEquals(dialog.getDialogPane().getHeight(), 
> dialog2.getDialogPane().getHeight(), 0.0);
> 69:         hide();
> 70:     }

When running the test on Linux without your fix, it gets the expected assertion 
failure, but then crashes. It looks like there is some interaction between the 
dialog still being open and the eventual call to `stage.hide()` and 
`Platform.exit`. I'll file a separate bug for this, since it is unrelated to 
your fix (I can make it happen with or without your fix).

I recommend putting the call to `hide()` in a try ... finally.

-------------



PR: https://git.openjdk.java.net/jfx/pull/63

Reply via email to