On Fri, 15 Nov 2024 17:47:34 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Alexander Zvegintsev has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - test cleanup
>> - add missing copyright header
>
> tests/system/src/test/java/test/robot/javafx/embed/swing/LinuxScreencastHangCrashTest.java
> line 60:
>
>> 58: Assumptions.assumeTrue(!Util.isOnWayland()); // JDK-8335470
>> 59: Assumptions.assumeTrue(Util.isOnWayland());
>> 60: robot = new Robot();
>
> This initializes the AWT toolkit before the FX toolkit. Have you also tested
> it the other way around? I'm not sure it matters on Linux (unlike macOS where
> it does matter), so maybe unimportant.
Yes, and I see no difference in behavior.
> tests/system/src/test/java/test/robot/javafx/embed/swing/LinuxScreencastHangCrashTest.java
> line 72:
>
>> 70: private static void awtPixelOnFxThread() throws InterruptedException
>> {
>> 71: System.out.println("awtPixelOnFxThread");
>> 72: initFX();
>
> Have you considered moving the `initFX()` call to the `init` method (after
> the call to AWT robot) so you only need it in one place?
It is intentionally kept out of the `init()` method to be able to test the
> 1. If there is no GTK main loop running
Example: just a JDK only application.
In this case we call g_main_context_iteration(NULL, TRUE) as before (when
[gtk_main_level() == 0](https://docs.gtk.org/gtk3/func.main_level.html)).
like
awtPixel();
robot.delay(DELAY_WAIT_FOR_SESSION_TO_CLOSE);
initFX();
robot.delay(500);
awtPixel();
> tests/system/src/test/java/test/robot/javafx/embed/swing/LinuxScreencastHangCrashTest.java
> line 143:
>
>> 141: DELAY_BEFORE_SESSION_CLOSE + 25
>> 142: ).forEach(delay -> {
>> 143: System.out.println("Testing with delay: " + delay);
>
> You might consider making this a parameterized test with this list of delays
> as the parameters, if it isn't too hard.
Sure, it is not hard at all. Updated.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1639#discussion_r1845231679
PR Review Comment: https://git.openjdk.org/jfx/pull/1639#discussion_r1845232139
PR Review Comment: https://git.openjdk.org/jfx/pull/1639#discussion_r1845232430
