RFR: 8356061: [macos] com/apple/laf/RootPane/RootPaneDefaultButtonTest.java test fails on macosx-aarch64 machine [v2]
Jeremy Wood
duke at openjdk.org
Thu May 15 19:31:52 UTC 2025
On Thu, 15 May 2025 17:21:37 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:
>> Jeremy Wood has updated the pull request incrementally with two additional commits since the last revision:
>>
>> - 8356061: restructuring to move things to EDT
>>
>> This was requested here:
>> https://github.com/openjdk/jdk/pull/25244#discussion_r2091656355
>>
>> mrserb asked: "Also please move creation and access_to all Swing components to EDT"
>>
>> (I'm not sure this will help much? My understanding was *creation* of Swing components could happen off the EDT as long as they were made displayable on the EDT.)
>>
>> Now we still call jc.getLocationOnScreen off the EDT. If that posed a thread-based problem it'd probably manifest as a IllegalComponentStateException, which is not mentioned in 8356061.
>> - 8356061: adding 1000ms pause after window construction
>>
>> This was requested here:
>> https://github.com/openjdk/jdk/pull/25244#discussion_r2091656355
>>
>> So now we'll pause at least 1.1s before the first call to `jc.getLocationOnScreen`, and at least 2.1s before the first call to `robot.getPixelColor(x, y)`.
>>
>> (getLocationOnScreen has never failed with an IllegalStateException.)
>
> test/jdk/com/apple/laf/RootPane/RootPaneDefaultButtonTest.java line 131:
>
>> 129: robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK);
>> 130:
>> 131: robot.delay(1000);
>
> it will be useful to add the same delay before the first call to test, right after invokeAndWait.....setVisible(...)
> Also please move creation and access_to all Swing components to EDT(button, radioButton1)
I added an additional delay. I'm not sure exactly what refactor you're looking for, but I rewrote the test to put more in the EDT.
(I double-checked that the test still passes, but that doesn't prove much since I've never reproduced a failure.)
I did try at one point blocking the testing thread until the EDT had a chance to complete an invokeLater runnable, but that didn't seem to make a difference so I didn't commit it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25244#discussion_r2091836908
More information about the client-libs-dev
mailing list