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

Khalid Boulanouare duke at openjdk.org
Mon Sep 15 08:42:02 UTC 2025


On Fri, 5 Sep 2025 11:37:57 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> 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
>
> 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.

Removed.

> 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`.

Done.

> 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.

Done.

> 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.

Done.

> 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`.

Done.

> 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.

Done!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2348237275
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2348267313
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2348226533
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2348217879
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2348232783
PR Review Comment: https://git.openjdk.org/jdk/pull/25971#discussion_r2348221815


More information about the client-libs-dev mailing list