RFR: JDK-8328190 : Convert AWTPanelSmoothWheel.html applet test to main [v8]

Fri Mar 22 15:35:25 UTC 2024

On Thu, 21 Mar 2024 19:53:48 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:

>> This test is converted to main using PassFailJFrame. It verifies wheel rotation value for high-res mouse on windows.
>> 
>> The test requires the updated PassFailJFrame's logArea() feature added in this PR https://github.com/openjdk/jdk/pull/18319
>
> Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   changed the forcePass logic, updated instructions

It looks good to me.

test/jdk/java/awt/event/MouseEvent/AWTPanelSmoothWheel.java line 52:

> 50:             <body>
> 51:             This test is relevant on platforms with high-resolution mouse wheel,
> 52:             please press PASS if this is not the case.<br> <br>

It may be worth mentioning that trackpad will suit too.

test/jdk/java/awt/event/MouseEvent/AWTPanelSmoothWheel.java line 68:

> 66:             an event where wheelRotation != 0 in the logs.<br> <br>
> 67: 
> 68:             Check if the test works OK when the mouse wheel is rotated very slow.<br> <br>

Suggestion:

            Check if the test works OK when the mouse wheel is rotated very slowly.<br> <br>

test/jdk/java/awt/event/MouseEvent/AWTPanelSmoothWheel.java line 69:

> 67: 
> 68:             Check if the test works OK when the mouse wheel is rotated very slow.<br> <br>
> 69:             This is a semi-automated test, if you are using a hi-res mouse and

I would move this paragraph above. At this moment, it's more important to say that the test will pass automatically than that the tester needs to look through the log of events. So, the sentence above could be removed altogether.

test/jdk/java/awt/event/MouseEvent/AWTPanelSmoothWheel.java line 74:

> 72: 
> 73:             <hr>
> 74:             PLEASE NOTE:

Suggestion:

            <b>Note:</b>

You have formatting options, bold text will stand out and it's usually easier to read than uppercase text.

test/jdk/java/awt/event/MouseEvent/AWTPanelSmoothWheel.java line 84:

> 82:                  and they are negative when scrolled down. </li>
> 83:             </ul>
> 84:             <br>

Suggestion:

May be removed.

test/jdk/java/awt/event/MouseEvent/AWTPanelSmoothWheel.java line 118:

> 116:                     PassFailJFrame.log(WARNING_MSG);
> 117:                     JOptionPane.showMessageDialog(frame, WARNING_MSG,
> 118:                             "WARNING", WARNING_MESSAGE);

Suggestion:

                            "WARNING", JOptionPane.WARNING_MESSAGE);

I suggest using `JOptionPane.WARNING_MESSAGE` explicitly: there's no confusion between `WARNING_MSG` and `WARNING_MESSAGE` then.

I'd use ‘Warning’ in its title case as opposed to uppercase, taking into account the message itself contains upper case warning.

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

Marked as reviewed by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18312#pullrequestreview-1955102847
PR Review Comment: https://git.openjdk.org/jdk/pull/18312#discussion_r1535779263
PR Review Comment: https://git.openjdk.org/jdk/pull/18312#discussion_r1535766207
PR Review Comment: https://git.openjdk.org/jdk/pull/18312#discussion_r1535769619
PR Review Comment: https://git.openjdk.org/jdk/pull/18312#discussion_r1535770843
PR Review Comment: https://git.openjdk.org/jdk/pull/18312#discussion_r1535773725
PR Review Comment: https://git.openjdk.org/jdk/pull/18312#discussion_r1535777940