On Fri, 6 Jan 2023 07:21:12 GMT, Manukumar V S <m...@openjdk.org> wrote:
>> java/awt/PopupMenu/PopupMenuLocation.java seems to be unstable in MacOS >> machines, especially in MacOSX 12 & 13 machines. It seems to be a testbug as >> adding some stability improvements fixes the issue. It intermittently fails >> in CI causing some noise. This test was already problem listed in windows >> due to an umbrella bug JDK-8238720. This fix doesn't cover the Windows >> issue, so the problem listing in windows will remain same. >> >> Fix: >> Some stability improvements have been done and the test has been run 100 >> times per platform in mach5 and got full PASS. >> Also updated the screen capture code, made frame undecorated, added some >> prints for better debugging. > > Manukumar V S has updated the pull request incrementally with one additional > commit since the last revision: > > Review comments fixed: Replaced one robot.waitForIdle with robot.delay(), > formatting corrections, taken the insets into account for initial frame > position Changes requested by aivanov (Reviewer). test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 77: > 75: while (point.x < > 76: screenBounds.x + screenBounds.width - insets.right > - > 77: SIZE) { Suggestion: final int yBound = screenBounds.y + screenBounds.height - insets.bottom - SIZE; final int xBound = screenBounds.x + screenBounds.width - insets.right - SIZE; while (point.y < yBound) { while (point.x < xBound) { I think this looks clearer and more readable than the wrapped condition. test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 87: > 85: } > 86: > 87: private static void test(final Point tmp) { Suggestion: private static void test(final Point loc) { Let's give a better name to the parameter. test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 104: > 102: frame.setSize(SIZE, SIZE); > 103: frame.setVisible(true); > 104: frame.setLocation(tmp.x, tmp.y); Suggestion: frame.setLocation(tmp); frame.setVisible(true); You can pass a `Point` object to `setLocation` directly. Setting the location before making the frame visible avoids flickering where the frame appears at 0, 0 and then moves to another location. ------------- PR: https://git.openjdk.org/jdk/pull/10655