-
Notifications
You must be signed in to change notification settings - Fork 5.8k
JDK-8320113: [macos14] : ShapeNotSetSometimes.java fails intermittently on macOS 14 #17180
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
Conversation
👋 Welcome back honkar! A progress list of the required criteria for merging this PR into |
@honkar-jdk The following label will be automatically applied to this pull request:
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. |
Webrevs
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this locally on my macOS 14.1.2 device and the test DID intermittently fail without the fix at different attempt #'s. But, with the updated test, the test is passing every time. Ran the test 10x and still passes. Stability fix looks good to me
@honkar-jdk 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:
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 287 new commits pushed to the
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
This test is failing intermittently on linux with the current fix. Running the test on all platforms with slightly different delay values. |
In this test CI Testing with the stabilization fix looks good on all platforms including macOS 14.
|
@azuev-java @azvegint @DamonGuy Please re-review the updated test stabilization fix. |
@@ -124,8 +125,9 @@ public void paint(Graphics g) { | |||
public static void main(String[] args) throws Exception { | |||
robot = new Robot(); | |||
|
|||
for(int i = 0; i < 50; i++) { | |||
for(int i = 1; i <= 50; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for(int i = 1; i <= 50; i++) { | |
for (int i = 1; i <= 50; i++) { |
Looks better, not an official reviewer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested the updated test changes. Still working as expected with 0 fails. Can I get some insight for the numOfFailedAttempts count being more or less than 3? Why is 3 significant?
As explained here earlier, I'm unable to reproduce the intermittent issue on CI testing (as before on linux with the earlier fix) but taking into account that this is an intermittent issue, I believe a 3/50 failed attempts is less strict and prevents the test to fail instantly due to glitches during any given attempt. A 5 or 10 failed attempts might be relaxing the constraint too much hence 3 was chosen. |
I found out from our offline discussion that it failed only on Linux and not on other platforms. Since it's very intermittent failure, and the test hasn't been on the problem list since April, and there have been no reported incidents on Linux since then, I think we should not hide such failures behind additional attempts until we find out the cause. I suggest adding some additional debugging information, such as saving a screenshot of what actually happened before the exception was thrown. |
Test updated according to @azvegint suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test updated according to @azvegint suggestions.
Thank you!
/integrate |
Going to push as commit f1b7335.
Your commit was automatically rebased without conflicts. |
@honkar-jdk Pushed as commit f1b7335. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
ShapeNotSetSometimes.java fails intermittently on macOS 14. Following is a test stabilization fix for macOS 14. With the added delays ShapeNotSetSometimes.java passes on older versions of macOS as well as on macOS 14.1, 14.2.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/17180/head:pull/17180
$ git checkout pull/17180
Update a local copy of the PR:
$ git checkout pull/17180
$ git pull https://git.openjdk.org/jdk.git pull/17180/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 17180
View PR using the GUI difftool:
$ git pr show -t 17180
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17180.diff
Webrev
Link to Webrev Comment