Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 40 commits into from

Conversation

manukumarvs
Copy link
Member

@manukumarvs manukumarvs commented Oct 7, 2022

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.


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8288415: java/awt/PopupMenu/PopupMenuLocation.java is unstable in MacOS machines

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10606/head:pull/10606
$ git checkout pull/10606

Update a local copy of the PR:
$ git checkout pull/10606
$ git pull https://git.openjdk.org/jdk pull/10606/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10606

View PR using the GUI difftool:
$ git pr show -t 10606

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10606.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Oct 7, 2022

👋 Welcome back mvs! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr Pull request is ready for review label Oct 7, 2022
@openjdk
Copy link

openjdk bot commented Oct 7, 2022

@manukumarvs The following label will be automatically applied to this pull request:

  • client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the client client-libs-dev@openjdk.org label Oct 7, 2022
@mlbridge
Copy link

mlbridge bot commented Oct 7, 2022

Webrevs

Dimension screenSize = Toolkit.getDefaultToolkit().getScreenSize();
try {
ImageIO.write(robot.createScreenCapture(
new Rectangle(0, 0, screenSize.width, screenSize.height)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may capture the default screen while the frame is currently visible on another one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, is there any way we can capture the current screen where the frame is displayed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass coordinates/bounds of that screen to the createScreenCapture.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do that in a new PR as something went wrong with this and some unnecessary commits got added

Alan Bateman and others added 23 commits October 11, 2022 18:50
…core system

Reviewed-by: sspitsyn, cjplummer
…sourceObj

Reviewed-by: coleenp, stefank, kbarrett
Reviewed-by: ihse, dnsimon, yadongwang
… "ERROR: Injection failure: java.lang.UnsupportedOperationException: Records requires ASM8"

Reviewed-by: lmesnik, cjplummer, sspitsyn
…allocator or Cleaner fails with OOME

Reviewed-by: alanb
…not unloading

Reviewed-by: dholmes, coleenp, sspitsyn
Reviewed-by: jnimeh, ascarpino
… may have a problem

Reviewed-by: sspitsyn, dcubed
Reviewed-by: xuelei, valeriep, wetmore
…e VMs after JDK-8294839

Reviewed-by: chagedorn, epeter
…ch5 for WIndowLookAndFeel

Reviewed-by: aivanov, tr
…h "Expected two batches of Active Setting events"

Reviewed-by: egahlin
… of exceptions whose documentation is inherited

Reviewed-by: jjg
egahlin and others added 16 commits October 11, 2022 18:50
…est failed on SVE machine

Reviewed-by: aph, njian
…ed value of shouldCommit()

Reviewed-by: mgronlun
…obox and selection textarea.

Reviewed-by: jdv, tr, psadhukhan
@aivanov-jdk
Copy link
Member

You did something wrong, now there are 40 commits and 126 files changed. It doesn't look right.

Merging master should result in one commit only even though it brings multiple commits into the history.

@manukumarvs
Copy link
Member Author

You did something wrong, now there are 40 commits and 126 files changed. It doesn't look right.

Merging master should result in one commit only even though it brings multiple commits into the history.

Yea, I have seen that. I tried to fix it, but its not working, will delete this branch and create a new one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client client-libs-dev@openjdk.org rfr Pull request is ready for review