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

8285094: Test java/awt/Frame/InvisibleOwner/InvisibleOwner.java failing on Linux #8312

Closed
wants to merge 4 commits into from

Conversation

prrace
Copy link
Contributor

@prrace prrace commented Apr 20, 2022

As discussed in https://bugs.openjdk.java.net/browse/JDK-8285094 it seems that the test
java/awt/Frame/GetGraphicsStressTest/GetGraphicsStressTest.java destabilises the Xserver.
This causes java/awt/Frame/InvisibleOwner/InvisibleOwner.java to fail and even before that
other tests which do pass are not visibly behaving as expected.

I didn't find any Xserver logs of "bad stuff" happening so it just seems like the Xserver was
having trouble keeping up and JDK was behaving correctly as it could despite the Xserver sending
lots of requests to repaint.

Just the "sleep" at the end of GetGraphicsStressTest.java seems to be enough but I'd already
reworked InvisibleOwner.java and I'd like to think it is a bit better than before.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8285094: Test java/awt/Frame/InvisibleOwner/InvisibleOwner.java failing on Linux

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/8312/head:pull/8312
$ git checkout pull/8312

Update a local copy of the PR:
$ git checkout pull/8312
$ git pull https://git.openjdk.java.net/jdk pull/8312/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8312

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8312.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Apr 20, 2022

👋 Welcome back prr! 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 changed the title 8285094 8285094: Test java/awt/Frame/InvisibleOwner/InvisibleOwner.java failing on Linux Apr 20, 2022
@openjdk
Copy link

openjdk bot commented Apr 20, 2022

@prrace 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 Apr 20, 2022
@openjdk openjdk bot added the rfr Pull request is ready for review label Apr 20, 2022
@mlbridge
Copy link

mlbridge bot commented Apr 20, 2022

Webrevs

Copy link
Contributor

@prsadhuk prsadhuk left a comment

Choose a reason for hiding this comment

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

Maybe a Thread.sleep(1000) is also needed after frame.setVisible(true); and also frame.dispose() as test() is called 4 times so we will give time to setup test after frame is visible and frame is disposed at test completion.

@prrace
Copy link
Contributor Author

prrace commented Apr 21, 2022

Maybe a Thread.sleep(1000) is also needed after frame.setVisible(true); and also frame.dispose() as test() is called 4 times so we will give time to setup test after frame is visible and frame is disposed at test completion.

I did not find this to be necessary. As I wrote putting a sleep at the beginning of InvisibleOwner also cures this so I am sure it is just a matter of letting things settle down AFTER the test.

@prrace prrace closed this Apr 21, 2022
@prrace prrace reopened this Apr 21, 2022
// helper frame, not the invisible frame.
robot.mouseMove(C_X, C_Y);
robot.mousePress(InputEvent.BUTTON1_MASK);
robot.mouseRelease(InputEvent.BUTTON1_MASK);
Copy link
Contributor

Choose a reason for hiding this comment

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

In other places,we have modified to use BUTTON1_DOWN_MASK, maybe we can do here too.

@openjdk
Copy link

openjdk bot commented Apr 28, 2022

@prrace This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8285094: Test java/awt/Frame/InvisibleOwner/InvisibleOwner.java failing on Linux

Reviewed-by: psadhukhan, serb

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 164 new commits pushed to the master branch:

  • 573eace: 8285504: Minor cleanup could be done in javax.net
  • bba456a: 8285676: Add missing @param tags for type parameters on classes and interfaces
  • b9d1e85: 8285785: CheckCleanerBound test fails with PasswordCallback object is not released
  • b718578: 8285011: gc/arguments/TestUseCompressedOopsFlagsWithUlimit.java fails after JDK-8280761
  • 2d8d140: 8285690: CloneableReference subtest should not throw CloneNotSupportedException
  • ea83b44: 8280510: AArch64: Vectorize operations with loop induction variable
  • 36bf6fb: 8285728: Alpine Linux build fails with busybox tar
  • 091637c: 8285630: Fix a configure error in RISC-V cross build
  • ccf0e8b: 8285755: JDK-8285093 changed the default for --with-output-sync
  • d7514b0: 8285595: Assert frame anchor doesn't change in safepoints/handshakes
  • ... and 154 more: https://git.openjdk.java.net/jdk/compare/c63fabe3d582ce0828b04b0224cea49aab5fedf3...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Apr 28, 2022
@@ -27,7 +27,7 @@

/**
* @test
* @bug 8235638 8235739
* @bug 8235638 8235739 8285094
Copy link
Member

Choose a reason for hiding this comment

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

It is not necessary to add bugid here, since this list is useful to verify the old bugs if the test will be reworked later.

@prrace
Copy link
Contributor Author

prrace commented Apr 29, 2022

/integrate

@openjdk
Copy link

openjdk bot commented Apr 29, 2022

Going to push as commit 64d98ba.
Since your change was applied there have been 164 commits pushed to the master branch:

  • 573eace: 8285504: Minor cleanup could be done in javax.net
  • bba456a: 8285676: Add missing @param tags for type parameters on classes and interfaces
  • b9d1e85: 8285785: CheckCleanerBound test fails with PasswordCallback object is not released
  • b718578: 8285011: gc/arguments/TestUseCompressedOopsFlagsWithUlimit.java fails after JDK-8280761
  • 2d8d140: 8285690: CloneableReference subtest should not throw CloneNotSupportedException
  • ea83b44: 8280510: AArch64: Vectorize operations with loop induction variable
  • 36bf6fb: 8285728: Alpine Linux build fails with busybox tar
  • 091637c: 8285630: Fix a configure error in RISC-V cross build
  • ccf0e8b: 8285755: JDK-8285093 changed the default for --with-output-sync
  • d7514b0: 8285595: Assert frame anchor doesn't change in safepoints/handshakes
  • ... and 154 more: https://git.openjdk.java.net/jdk/compare/c63fabe3d582ce0828b04b0224cea49aab5fedf3...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Apr 29, 2022
@openjdk openjdk bot closed this Apr 29, 2022
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 29, 2022
@openjdk
Copy link

openjdk bot commented Apr 29, 2022

@prrace Pushed as commit 64d98ba.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

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 integrated Pull request has been integrated
4 participants