On Thu, 21 Mar 2024 19:53:48 GMT, Harshitha Onkar <hon...@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
