RFR: 8340173: Open source some Component/Panel/EventQueue tests - Set2 [v2]

Harshitha Onkar honkar at openjdk.org
Tue Oct 8 22:42:59 UTC 2024


On Tue, 8 Oct 2024 16:55:19 GMT, Damon Nguyen <dnguyen at openjdk.org> wrote:

>> Following tests are added as part of this PR:
>> 
>> 1. /java/awt/LightweightComponent/MultipleAddNotifyTest/MultipleAddNotifyTest.java - CONVERTED TO AUTO
>> 2. /java/awt/LightweightComponent/PopupTest/PopupTest.java - MANUAL
>> 3. /java/awt/Panel/PanelRepaint/PanelRepaint.java - MANUAL
>> 4. /java/awt/EventQueue/PushPopDeadlock/PushPopDeadlock.java MANUAL (PROBLEM LISTED)
>
> Damon Nguyen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments

test/jdk/java/awt/EventQueue/PushPopDeadlock/PushPopDeadlock.java line 59:

> 57:                 .testUI(PushPopDeadlock::createUI)
> 58:                 .build()
> 59:                 .awaitAndCheck();

Though the test is problemlisted I think it might be better to separate the createUI code from the EventQueue push/pop test logic as below. 
With the test() method having all the code from  `EventQueue q = new EventQueue() {  ....` onwards.


 PassFailJFrame pfJFrame = PassFailJFrame.builder()
                                          .title("Test Instructions")
                                          .instructions(INSTRUCTIONS)
                                          .columns(35)
                                          .testUI(PushPopDeadlock::createUI)
                                          .build();
PushPopDeadlock.test();
pfJFrame.awaitAndCheck();

test/jdk/java/awt/EventQueue/PushPopDeadlock/PushPopDeadlock.java line 65:

> 63:         Frame f = new Frame("Click Here!");
> 64: 
> 65:         Label l = new Label("" + counter);

Adding a label to counter seems better

Suggestion:

        Label l = new Label("Counter" + counter);

test/jdk/java/awt/EventQueue/PushPopDeadlock/PushPopDeadlock.java line 90:

> 88:                     return;
> 89:                 }
> 90:             }

This is a busy wait loop, maybe there is a better way of doing it although it might also be out-of-scope for this PR changes.

test/jdk/java/awt/Panel/PanelRepaint/PanelRepaint.java line 28:

> 26:  * @bug 4148078
> 27:  * @summary Repainting problems in scrolled panel
> 28:  * @library /open/test/jdk/java/awt/regtesthelpers

library path

test/jdk/java/awt/Panel/PanelRepaint/PanelRepaint.java line 50:

> 48: 
> 49: public class PanelRepaint extends Panel implements FocusListener {
> 50:     static Frame frame;

unused variable

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21398#discussion_r1792581692
PR Review Comment: https://git.openjdk.org/jdk/pull/21398#discussion_r1792581777
PR Review Comment: https://git.openjdk.org/jdk/pull/21398#discussion_r1792585101
PR Review Comment: https://git.openjdk.org/jdk/pull/21398#discussion_r1792566503
PR Review Comment: https://git.openjdk.org/jdk/pull/21398#discussion_r1792568114


More information about the client-libs-dev mailing list