RFR: 8343736: Test java/awt/Focus/UnaccessibleChoice/AccessibleChoiceTest.java failed: Choice can't be controlled by keyboard [v2]

[Damon Nguyen]

On Tue, 26 Nov 2024 07:04:45 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:

>> Damon Nguyen has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - String comparison
>>  - Review comments
>
> test/jdk/java/awt/Focus/UnaccessibleChoice/AccessibleChoiceTest.java line 60:
> 
>> 58:     public static void main(final String[] args) throws Exception {
>> 59:         AccessibleChoiceTest app = new AccessibleChoiceTest();
>> 60:         app.test();
> 
> Since you are doing a bit of modifictaion to the test., you may think of modifying it further.
> 1. Don't think you need to create the object.
> 2. UI creation code can be moved to more meaningful method name `createAndShowUI instead of init` inside EDT block and followed by call to `test` method.

Sure. It cleans the test up. Wasn't in my initial plan, but it's a good point. Done.

> test/jdk/java/awt/Focus/UnaccessibleChoice/AccessibleChoiceTest.java line 102:
> 
>> 100: 
>> 101:         // Focus default button and wait till it gets focus
>> 102:         Point loc = def.getLocationOnScreen();
> 
> Should be accessed on EDT?
> Frame and other UI components also should be created on EDT ?

Added. Had to make point volatile and static in the process.

> test/jdk/java/awt/Focus/UnaccessibleChoice/AccessibleChoiceTest.java line 129:
> 
>> 127:         }
>> 128: 
>> 129:         // Press Down key to select next item in the choice(Motif 2.1)
> 
> Motif is deprecated.. can be removed
> Suggestion:
> 
>         // Press Down key to select next item in the choice

Done

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22333#discussion_r1861394395
PR Review Comment: https://git.openjdk.org/jdk/pull/22333#discussion_r1861392666
PR Review Comment: https://git.openjdk.org/jdk/pull/22333#discussion_r1861392517