RFR: 8288415: java/awt/PopupMenu/PopupMenuLocation.java is unstable in MacOS machines

[Damon Nguyen]

On Tue, 11 Oct 2022 14:17:42 GMT, Manukumar V S <mvs at 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.

I ran the updated test in CI and locally, and it seems to be OK. My issue is that when I run locally, the test takes noticeably longer to execute and complete. I tried investigating the issue by implementing your changes incrementally, and I believe it's due to the waitForIdle after button3 presses. Maybe it's better to place a set amount of delay in ms here instead? Additionally, there are multiple spacing issues in the updated lines, and the header needs to be updated to have "2022" alongside the "2017" test date.

test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 75:

> 73:             while (point.y < currentScreenBounds.y + currentScreenBounds.height - insets.bottom - SIZE) {
> 74:                 while (point.x <
> 75:                            currentScreenBounds.x + currentScreenBounds.width - insets.right - SIZE) {

This might not be the most elegant way to abide by the rule to create a new line for long lines. I believe the line limit is 80 characters, which this still violates. And since it looks like you're trying to fix that here, might as well do it correctly.

test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 114:

> 112:                 private void show(MouseEvent e) {
> 113:                     if (e.isPopupTrigger()) {
> 114:                         System.out.println("Going to show popup "+pm+" on "+frame);

There seems to be a lack of consistency with spacing throughout the test. Spacing out the "+" from the quotations and the vars makes it cleaner.

test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 133:

> 131:         robot.mousePress(InputEvent.BUTTON3_DOWN_MASK);
> 132:         robot.mouseRelease(InputEvent.BUTTON3_DOWN_MASK);
> 133:         robot.waitForIdle();

When testing locally on my macOS machine, this waitForIdle seems to be the culprit for why this test takes much longer per iteration. Maybe worth looking into why?

test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 134:

> 132:         robot.mouseRelease(InputEvent.BUTTON3_DOWN_MASK);
> 133:         robot.waitForIdle();
> 134:          y = y+50;

Another instance of the spacing issue. There seems to be more in this test, so it's probably best to fix them all.

test/jdk/java/awt/PopupMenu/PopupMenuLocation.java line 151:

> 149:         try {
> 150:             ImageIO.write(robot.createScreenCapture(currentScreenBounds), "png",
> 151:                           new File("screen1.png"));

Is screen1.png the best name for the image? May be misleading if there are multiple screens, especially since this test iterates multiple locations.

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

PR: https://git.openjdk.org/jdk/pull/10655