-
Notifications
You must be signed in to change notification settings - Fork 5.9k
8334170: bug6492108.java test failed with exception Image comparison failed at (0, 0) for image 4 #19788
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 abhiscxk! A progress list of the required criteria for merging this PR into |
@kumarabhi006 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 9 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
@kumarabhi006 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. |
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.
You should ask Vitaly to test your changeset in his environment to confirm the failure is gone.
setDelay(50); | ||
return panel; |
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.
How does it help? You're delaying EDT.
You should rather call setDelay(50)
to add the delay between method calls.
Alternatively, you can add -delay 50
to the test arguments in its @run
tag.
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.
How does it help? You're delaying EDT.
I was unable to reproduce the failure scenario in my local machine but didn't observe any failure in mach5 also. Will ask Vitaly or @azvegint to verify as they are able to replicate the failure.
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.
You should rather call setDelay(50) to add the delay between method calls.
this change does exactly that, it calls SwingTestHelper#setDelay
since bug6492108 extends SwingTestHelper
You should ask Vitaly to test your changeset in his environment to confirm the failure is gone.
I am able to reproduce the issue locally on Ubuntu 22.04, and the provided fix works fine for me.
I assume @vprovodin has already done this testing, since he was the one who provided the solution in the JBS issue description.
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.
My bad, I read it as if it were delay(50)
.
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.
Would it be clearer if setDelay(50)
was called in the constructor of bug6492108
?
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.
Would it be clearer if setDelay(50) was called in the constructor of bug6492108?
I am ok with the current placing of setDelay(50).
setDelay(50); | ||
return panel; |
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.
You should rather call setDelay(50) to add the delay between method calls.
this change does exactly that, it calls SwingTestHelper#setDelay
since bug6492108 extends SwingTestHelper
You should ask Vitaly to test your changeset in his environment to confirm the failure is gone.
I am able to reproduce the issue locally on Ubuntu 22.04, and the provided fix works fine for me.
I assume @vprovodin has already done this testing, since he was the one who provided the solution in the JBS issue description.
setDelay(50); | ||
return panel; |
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.
My bad, I read it as if it were delay(50)
.
/integrate |
Going to push as commit 9ef86da.
Your commit was automatically rebased without conflicts. |
@kumarabhi006 Pushed as commit 9ef86da. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Test failed intermittently on Ubuntu 20.04, Ubuntu 22.04 system. Added a delay to stable the test and multiple run in CI is Ok. Link is added in JBS.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19788/head:pull/19788
$ git checkout pull/19788
Update a local copy of the PR:
$ git checkout pull/19788
$ git pull https://git.openjdk.org/jdk.git pull/19788/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 19788
View PR using the GUI difftool:
$ git pr show -t 19788
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19788.diff
Webrev
Link to Webrev Comment