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

[Alexey Ivanov]

On Wed, 3 Sep 2025 09:22:17 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 46 additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into jdk-8158801
>  - Disposes frame at the end of testing.
>  - Moves latch logic inside ancestor frame block
>  - Merge branch 'openjdk:master' into jdk-8158801
>  - Disposes frames after each AWT component test
>  - Removes redundant code for centring frames
>  - Merge branch 'openjdk:master' into jdk-8158801
>  - Merge branch 'openjdk:master' into jdk-8158801
>  - Centers missed frames in the middle of screen
>  - Uses KeyboardFocusManager instead of FocusManager
>  - ... and 36 more: https://git.openjdk.org/jdk/compare/e79df9f1...a62416a9

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/Mixing/AWT_Mixing/GlassPaneOverlappingTestBase.java line 63:

> 61:     {
> 62:         multiFramesTest = false;
> 63:     }

I prefer this is done in a constructor rather than a simple initialiser, it just makes the intent clearer.

The same applies to setting the initial value of `multiFramesTest` in `SimpleOverlappingTestBase`.

`SimpleOverlappingTestBase` has constructors, `GlassPaneOverlappingTestBase` has constructors too. I believe `multiFramesTest` doesn't change, and if you initialise it in a constructor, you can make it `final`, and I'm always for using immutable fields.

test/jdk/java/awt/Mixing/AWT_Mixing/GlassPaneOverlappingTestBase.java line 125:

> 123:         final CountDownLatch latch = new CountDownLatch(1);
> 124:         f.addFocusListener(new FocusAdapter() {
> 125:             @Override public void focusGained(FocusEvent e) {

Suggestion:

        f.addFocusListener(new FocusAdapter() {
            @Override
            public void focusGained(FocusEvent e) {

I prefer a multi-line version of this.

test/jdk/java/awt/Mixing/AWT_Mixing/GlassPaneOverlappingTestBase.java line 129:

> 127:             }
> 128:         });
> 129:         if (testResize) {

Suggestion:

        });

        if (testResize) {

Let's add a blank line to introduce logical blocks in this long piece of code.

test/jdk/java/awt/Mixing/AWT_Mixing/GlassPaneOverlappingTestBase.java line 145:

> 143:                         } else {
> 144:                             f.requestFocusInWindow();
> 145:                         }

Is it possible to add the listener earlier to ensure the latch is released in the focus listener?

test/jdk/java/awt/Mixing/AWT_Mixing/GlassPaneOverlappingTestBase.java line 148:

> 146:                     }
> 147:                 });
> 148:             } catch (InterruptedException ex) {

Although this is existing code, I propose merging the two catch block into one by using `InterruptedException | InvocationTargetException` as the type.

test/jdk/java/awt/Mixing/AWT_Mixing/GlassPaneOverlappingTestBase.java line 165:

> 163:             return wasLWClicked;
> 164:         } else {
> 165:             latch.countDown();

I believe this is not required, the test will not block even if the latch isn't released.

test/jdk/java/awt/Mixing/AWT_Mixing/GlassPaneOverlappingTestBase.java line 169:

> 167:         }
> 168:     }
> 169:     protected void cleanup(){

Suggestion:

    }

    @Override
    protected void cleanup(){


Add a blank line between methods, and add the `@Override` annotation.

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

> 1: /*

Remove `import java.awt.Color;`

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

> 1: /*

Remove `import java.awt.Color;`

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

> 1: /*

Could you expand the wildcard imports, please?

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

> 1: /*

Could you also move `TestPassedException` inside `MixingPanelsResizing` and make it `static`?

This would resolve multiple definitions of `TestPassedException` when viewing source for AWT_Mixing in an IDE.

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

> 1: /*

Could you also move `TestPassedException` inside `OverlappingTestBase` and make it `static`?

This would resolve multiple definitions of `TestPassedException` when viewing source for AWT_Mixing in an IDE.

test/jdk/java/awt/Mixing/AWT_Mixing/OverlappingTestBase.java line 264:

> 262:                 embedder.setPreferredSize(new Dimension(150, 150));
> 263:                 container.add(embedder);
> 264:                 if(container instanceof Window){

Suggestion:

                if (container instanceof Window){

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

PR Review: https://git.openjdk.org/jdk/pull/25971#pullrequestreview-3184521964
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321616385
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321595814
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321636770
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321642154
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321646046
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321655342
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321652023
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321678210
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321679963
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321687965
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321703332
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321710616
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2321711250