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

Alexey Ivanov aivanov at openjdk.org
Thu Sep 4 14:33:54 UTC 2025


On Thu, 4 Sep 2025 13:20:32 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 one additional commit since the last revision:
> 
>   Organizes imports

Changes requested by aivanov (Reviewer).

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

> 26: import java.awt.event.MouseAdapter;
> 27: import java.awt.event.MouseEvent;
> 28: import java.lang.Override;

All the classes from `java.lang.*` are imported implicitly.

That is, remove `import java.lang.Override`.

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

> 98:     protected boolean isMultiFramesTest(){
> 99:         return false;
> 100:     }

There should be no blank line before the closing brace of a method or constructor. That is the snippet should like this:


    public GlassPaneOverlappingTestBase(boolean defaultClickValidation) {
        super(defaultClickValidation);
    }

    @Override
    protected boolean isMultiFramesTest() {
        return false;
    }

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

> 113: 
> 114:         if (testResize) {
> 115:             wasLWClicked = false;

May I also suggest reversing the condition:


       if (!testResize) {
          return true;
       }


This brings the shorter `else` section higher, and it's the end of the test if `testResize` is `false`.

The testing for resize then is written with reduced indentation.

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

> 313:         mainThread.interrupt();
> 314:     }//fail()
> 315:     static class TestPassedException extends RuntimeException {

Suggestion:

    }//fail()

    static class TestPassedException extends RuntimeException {

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

> 316:     }
> 317: }// class JButtonInGlassPane
> 318: 

Just one blank line is enough.

That is you should see only one blank line in your editor in the end of the file (some editors, like `vim`, hide this very last line).

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

> 690:     }//fail()
> 691:     static class TestPassedException extends RuntimeException {
> 692:     }

Suggestion:

    }//fail()

    static class TestPassedException extends RuntimeException {
    }

Add blank lines between class members, it improves readability, the code is easier to scan quickly with eyes.

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

> 692:     }
> 693: }// class LWComboBox
> 694: 

One blank line is enough.

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

> 53:     }
> 54: 
> 55:     protected final boolean MULTI_FRAMES_TEST ;

Since you introduced a method that returns whether the current instance is a multi-frame test or not, you don't need a field for this — just call the method instead of using a field, moreover it's used once only.

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

> 71:     }
> 72: 
> 73:     protected boolean isMultiFramesTest(){

Suggestion:

    protected boolean isMultiFramesTest() {

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

> 74:         return true;
> 75:     }
> 76:     public SimpleOverlappingTestBase() {

Suggestion:

    }

    public SimpleOverlappingTestBase() {

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

> 183: 
> 184:             clickAndBlink(robot, lLoc);
> 185:             if (ancestor != null && MULTI_FRAMES_TEST) {

Suggestion:

            if (ancestor != null && isMultiFramesTest()) {

If you introduce the method to return the flag and the flag is used in one place only, call the method instead of storing its value in field.

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

PR Review: https://git.openjdk.org/jdk/pull/25971#pullrequestreview-3185552759
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322334990
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322344365
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322371185
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322389259
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322380162
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322387677
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322384844
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322399919
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322396478
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322392789
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2322395914


More information about the client-libs-dev mailing list