On Tue, 3 Oct 2023 20:23:20 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Damon Nguyen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add setAutoWaitForIdle
>
> test/jdk/java/awt/dnd/RejectDragDropActionTest.java line 1:
>
>> 1: /*
>
> I've got questions whether all the AWT resources are released.
>
> When the test fails with timeout, the screen shot does not have the frame, it
> displays an empty desktop.
>
> The stack trace for the main thread:
>
>
> java.base/java.lang.Object.wait0(Native Method)
> java.base/java.lang.Object.wait(Object.java:375)
> java.base/java.lang.Thread.join(Thread.java:2045)
> java.base/java.lang.Thread.join(Thread.java:2121)
> java.base/java.lang.ApplicationShutdownHooks.runHooks(ApplicationShutdownHooks.java:114)
> java.base/java.lang.ApplicationShutdownHooks$1.run(ApplicationShutdownHooks.java:47)
> java.base/java.lang.Shutdown.runHooks(Shutdown.java:130)
> java.base/java.lang.Shutdown.exit(Shutdown.java:167)
> java.base/java.lang.Runtime.exit(Runtime.java:188)
> java.base/java.lang.System.exit(System.java:1920)
> com.sun.javatest.regtest.agent.AStatus.exit(AStatus.java:198)
> com.sun.javatest.regtest.agent.MainWrapper.main(MainWrapper.java:95)"
>
>
> That is the main method of the test class terminated. Yet JVM does not exit
> for whatever reason.
>
> At the same time, the thread dump contains both `AWT-Windows` and
> `AWT-EventQueue-0` threads, which means AWT prevents the JVM from exiting.
> Why? I have no clue.
>
> Will removing the mouse listeners from the frame let it be disposed of?
>
> if (frame != null) {
> frame.removeMouseListener((MouseListener) dgr);
> frame.removeMouseMotionListener((MouseMotionListener)
> dgr);
> frame.dispose();
> }
>
>
> Although the changes that you've made make the test faster, it may still time
> out unless we find the root cause.
Interesting, let me look into this quickly. Didn't notice this.
> test/jdk/java/awt/dnd/RejectDragDropActionTest.java line 89:
>
>> 87: try {
>> 88: Robot robot = new Robot();
>> 89: robot.setAutoWaitForIdle(true);
>
> Now that you set `autoWaitForIdle`, the delay inside the loop is redundant —
> waiting for idle after sending each event delays the execution even more.
Removed, thanks!
> test/jdk/java/awt/dnd/RejectDragDropActionTest.java line 91:
>
>> 89: robot.setAutoWaitForIdle(true);
>> 90: robot.waitForIdle();
>> 91: robot.delay(DELAY_TIME);
>
> To reduce the initial delay, you may override `Frame.paint` and `countDown` a
> latch after calling `super.paint`. Once the latch is released, you create
> robot and start testing.
>
> `DELAY_TIME` could be set to `500` or even less.
I reduced the `DELAY_TIME` to 500. Thanks for the suggestion. I got used to
setting the default delay times to 1000 for these types of tests, and am trying
to break the habit and reduce delays further to 500 where possible.
> test/jdk/java/awt/dnd/RejectDragDropActionTest.java line 99:
>
>> 97:
>> 98: robot.delay(DELAY_TIME);
>> 99:
>
> The delay here is unnecessary, nothing has changed after the initial delay.
Removed, thanks!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16018#discussion_r1344821938
PR Review Comment: https://git.openjdk.org/jdk/pull/16018#discussion_r1344824016
PR Review Comment: https://git.openjdk.org/jdk/pull/16018#discussion_r1344823824
PR Review Comment: https://git.openjdk.org/jdk/pull/16018#discussion_r1344823927
