RFR: 8340015: Open source several AWT focus tests - series 7 [v2]

[Prasanta Sadhukhan]

On Tue, 17 Sep 2024 23:40:49 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   MToolkit removed
>
> LGTM other than the minor inline review comments

@honkar-jdk please rereview..

> test/jdk/java/awt/Focus/MinimizeNonfocusableWindowTest.java line 59:
> 
>> 57:                 .instructions(INSTRUCTIONS)
>> 58:                 .rows((int) INSTRUCTIONS.lines().count() + 5)
>> 59:                 .columns(35)
> 
> Looks better formatted with the following:
> 
> Suggestion:
> 
>                 .rows((int) INSTRUCTIONS.lines().count() + 1)
>                 .columns(40)

ok

> test/jdk/java/awt/Focus/MinimizeNonfocusableWindowTest.java line 61:
> 
>> 59:                 .columns(35)
>> 60:                 .testUI(MinimizeNonfocusableWindowTest::createTestUI)
>> 61:                 .build()
> 
> Suggestion:
> `positionTestUI()` has been recently added to PassFailJFrame https://github.com/openjdk/jdk/pull/21023
> Since this test has multiple test UIs you can try the new option to position them if it doesn't require much re-work.

I will skip this one as it has already been positioned and I dont think the API will auto-position unlike testUI

> test/jdk/java/awt/Focus/MinimizeNonfocusableWindowTest.java line 66:
> 
>> 64: 
>> 65:     private static List<Window> createTestUI() {
>> 66:         Panel panel = new Panel();
> 
> Unused variable can be removed.

ok

> test/jdk/java/awt/Focus/WindowDisposeFocusTest.java line 68:
> 
>> 66: 
>> 67:     static public Window test(String[] args) {
>> 68:         final JFrame frame = new JFrame();
> 
> Frame title is missing. 
> Does setting a frame size look better here although the test is more to do with the dialogs?

ok

> test/jdk/java/awt/Focus/bug6435715.java line 78:
> 
>> 76:                 synchronized (this) {
>> 77:                     try {
>> 78:                         wait(3000);
> 
> Is large wait time required here? I think it can be reduced.

reduced..

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

PR Comment: https://git.openjdk.org/jdk/pull/21008#issuecomment-2357409777
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764335877
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764336544
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764335831
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764336061
PR Review Comment: https://git.openjdk.org/jdk/pull/21008#discussion_r1764335944