RFR: 8169475 : WheelModifier.java fails by timeout [v2]

Alexey Ivanov aivanov at openjdk.org
Mon Oct 23 17:53:41 UTC 2023


On Mon, 23 Oct 2023 03:28:00 GMT, Renjith Kannath Pariyangad <duke at openjdk.org> wrote:

>> Hi Reviewers,
>> 
>> I have added additional CountDownLatch for making sure button is visible before proceeding,(observe this was one of the key reason for failure) so this helps to improve test case stability. Modified _await()_ function by adding 1sec timeout with exception, this will help to reduce total execution time in case of failure. 
>> Please review and let me know your suggestions, if any.
>
> Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Added null check before frame dispose

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/event/MouseWheelEvent/WheelModifier/WheelModifier.java line 141:

> 139:         r.mouseMove(sLoc.x + bSize.width / 2, sLoc.y + bSize.height * 2);
> 140:         if (!exitSema.await(1, TimeUnit.SECONDS)) {
> 141:             throw new RuntimeException("Mouse is not moved");

Suggestion:

            throw new RuntimeException("Mouse did not exit");

test/jdk/java/awt/event/MouseWheelEvent/WheelModifier/WheelModifier.java line 148:

> 146:         r.mouseWheel(1);
> 147:         if (!wheelSema.await(1, TimeUnit.SECONDS)) {
> 148:             throw new RuntimeException("Mouse is not wheeled");

Suggestion:

            throw new RuntimeException("Mouse is not wheeled 1");

For consistency with the printed message.

test/jdk/java/awt/event/MouseWheelEvent/WheelModifier/WheelModifier.java line 174:

> 172:         } finally {
> 173:             if (test.f != null)
> 174:                 SwingUtilities.invokeAndWait(test.f::dispose);

This is not thread-safe.

Suggestion:

            SwingUtilities.invokeAndWait(() -> {
                if (test.f != null) {
                   test.f.dispose();
               }
            });

Please always use braces even when there's only one statement in the body.

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

PR Review: https://git.openjdk.org/jdk/pull/16281#pullrequestreview-1693154589
PR Review Comment: https://git.openjdk.org/jdk/pull/16281#discussion_r1369043388
PR Review Comment: https://git.openjdk.org/jdk/pull/16281#discussion_r1369043551
PR Review Comment: https://git.openjdk.org/jdk/pull/16281#discussion_r1369046750


More information about the client-libs-dev mailing list