On Tue, 11 Oct 2022 14:17:42 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.
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
