-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8361387 : Test bug4655513.java fails intermittently on Windows platform. #26824
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
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back abaya! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
@anass-baya 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
|
robot.mouseMove(dragStartLoc.x, y); | ||
robot.delay(50); | ||
robot.delay(20); | ||
} | ||
robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK); |
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.
mousePress parameter should be InputEvent.BUTTON1_DOWN_MASK
instead which is usually the norm? mouseRelease is called correctly..
Also, I dont think we usually use delay between drag...guess once drag is complete, we can call waitForIdle and delay..
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.
Hello @prsadhuk,
Thank you for your review.
its done
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.
Hello @prsadhuk,
We need to add at least waitForIdle() before releasing the mouse button. Otherwise, the test will start failing again.
KR,
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.
OK..Can you please run the test with repeat count of atleast 50 on all platforms and share the result in JBS?
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.
Hello @prsadhuk,
Linux is not stable with that one, while it is stable with my first proposed enhancement. Do you agree with rolling back to it?
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.
In the mainline or in other update releases?
I dont see this test fail in mainline except once due to NPE while disposing..
If using slight delay in between mouseMove works well in all trains for multiple iterations then you can add that I presume..
use InputEvent
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.
import java.awt.event.MouseEvent;
can be removed.
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 are you concluding this as timing issue ? Since this issue is intermittent.
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 agree. I tested with and without and I haven't seen it occur yet but I can run larger amounts of reruns. I'm not 100% sold that this will fix the intermittent issue but I don't see an issue with the update.
This test was recently automated and is failing intermittently in the CI due to timing issues.
This enhancement aims to stabilize the test and also adds the missing null pointer check.
Progress
Integration blocker
Warning
8361387: Test bug4655513.java fails intermittently on Windows platform.
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26824/head:pull/26824
$ git checkout pull/26824
Update a local copy of the PR:
$ git checkout pull/26824
$ git pull https://git.openjdk.org/jdk.git pull/26824/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 26824
View PR using the GUI difftool:
$ git pr show -t 26824
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26824.diff
Using Webrev
Link to Webrev Comment