On Thu, 5 May 2022 06:58:26 GMT, Srinivas Mandalika <smandal...@openjdk.org> wrote:
> Create an automated test for > [JDK-4516019](https://bugs.openjdk.java.net/browse/JDK-4516019) > > Clicking on the increment/decrement buttons of the spinner does not install > focus on the spinner or one of its focusable children. > > The test validates the same. > > This review is for migrating tests from a closed test suite to open. > > Testing: > The test ran successfully on Mach5 with multiple runs (30) on windows-x64, > linux-x64 and macos-x64. Changes requested by prr (Reviewer). test/jdk/javax/swing/4516019/JSpinnerFocusTest.java line 29: > 27: * @bug 4516019 > 28: * @summary Clicking on the increment/decrement buttons of the spinner > 29: * does not get focus onto the spinner. That summary is surely describing the bug, not what the test is verifying. Shouldn't it read something like "Verify clicking on inc/dec gives focus to the spinner" test/jdk/javax/swing/4516019/JSpinnerFocusTest.java line 81: > 79: "Clicking on JSpinner buttons did not" > 80: + " shift foucs to the JSpinner"); > 81: } foucs -> focus test/jdk/javax/swing/4516019/JSpinnerFocusTest.java line 100: > 98: robot.mouseMove(bounds.x + bounds.width/2 , > 99: bounds.y + bounds.height /2); > 100: robot.delay(300); why not use setAutoDelay(300) and then you don't need all these extra lines of code. test/jdk/javax/swing/4516019/JSpinnerFocusTest.java line 105: > 103: robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK); > 104: Thread.sleep(300); > 105: // Move cursor to click spinner up arrow button and why is this one (anomalously) Thread.sleep(..) ? test/jdk/javax/swing/4516019/JSpinnerFocusTest.java line 113: > 111: robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK); > 112: } catch (Exception e) { > 113: e.printStackTrace(); Shouldn't the test fail if we get an exception ? ------------- PR: https://git.openjdk.java.net/jdk/pull/8546
