RFR: 8158801: [TEST_BUG] Mixing tests fail because of focus workaround trick [v39]

Alexey Ivanov aivanov at openjdk.org
Fri Sep 5 11:46:24 UTC 2025


On Thu, 4 Sep 2025 22:02:14 GMT, Khalid Boulanouare <duke at openjdk.org> wrote:

>> Many Mixing tests failed because the work around click lands on the minimizing area in the window control and causes the tests to fail.
>> 
>> This fix changes the width of base frames which allows most of tests to pass.
>
> Khalid Boulanouare has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Cleans up code for move visibility
>  - Cleans up code and adds missed import

Minor formatting and unneeded imports left.

test/jdk/java/awt/Mixing/AWT_Mixing/JInternalFrameMoveOverlapping.java line 1:

> 1: /*

You may want to remove reference to an internal URL in the javadoc of this class… in another clean-up.

test/jdk/java/awt/Mixing/AWT_Mixing/JMenuBarOverlapping.java line 1:

> 1: /*

`java.awt.Color` can be removed from imports.

You may also add blank lines between import blocks as you did for `JPopupMenuOverlapping`.

test/jdk/java/awt/Mixing/AWT_Mixing/OpaqueOverlapping.java line 1:

> 1: /*

Line 59:


-        failMessage = "Opacity test mismatchs";
+        failMessage = "Opacity test mismatches";


Could be postponed, I just noticed it in the errors reported when I ran these tests locally.

test/jdk/java/awt/Mixing/AWT_Mixing/SimpleOverlappingTestBase.java line 30:

> 28: import java.awt.event.MouseAdapter;
> 29: import java.awt.event.MouseEvent;
> 30: import java.lang.Override;

Suggestion:


Not needed.

test/jdk/java/awt/Mixing/AWT_Mixing/SimpleOverlappingTestBase.java line 162:

> 160:         JFrame ancestor = (JFrame)(testedComponent.getTopLevelAncestor());
> 161: 
> 162:         if (ancestor != null) {

Suggestion:

        if (ancestor != null) {

I won't add a blank line here, the `if` is closely related to the variable above, `ancestor`.

test/jdk/java/awt/Mixing/AWT_Mixing/SimpleOverlappingTestBase.java line 172:

> 170:             try {
> 171:                 boolean await = latch.await(1L, TimeUnit.SECONDS);
> 172:                 if (!await) {

Suggestion:

                if (!latch.await(1L, TimeUnit.SECONDS)) {

I don't see any value in saving the result of `await` in a variable; the call itself isn't long enough; calling `latch.await` in the condition is a common pattern.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25971#pullrequestreview-3188970244
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2324860875
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2324850151
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2324869518
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2324829844
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2324833304
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2324838575


More information about the client-libs-dev mailing list