8333403: Write a test to check various components events are triggered properly #19521
Conversation
|
👋 Welcome back rgupta! A progress list of the required criteria for merging this PR into |
|
@ravigupta30 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 1303 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. As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@DamonGuy, @aivanov-jdk) but any other Committer may sponsor as well. ➡️ To flag this PR as ready for integration with the above commit message, type |
|
@ravigupta30 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
|
|
Copyright is missing. Please add the Copyright. |
There was a problem hiding this comment.
Tested on MacOS. The test passes and looks functional. Minor comments.
Have you tried on Windows with different scaling? Looks like the only component location you access is for the panel position, which should be safe. I tried different scalings on MacOS with no issue. Curious because this was an issue with JButton a while before.
|
@DamonGuy |
|
any volunteer for review? |
| }); | ||
|
|
||
| robot.delay(DELAY); | ||
| if (componentShown || componentHidden) { |
There was a problem hiding this comment.
Is it possible that any other event is triggered? Or do you expect other events?
When you inconify the frame, you verify of all the event flags but you verify only two flags here.
There was a problem hiding this comment.
Here we expect events should not triggered.
When inconify i expect all events should not triggered , but when form Frame.ICONIFIED to Frame.NORMAL componentMoved and componentResized events are triggered , Events order on Windows and Mac are different.
Order of the events for Frame from Frame.ICONIFIED to Frame.NORMAL
On Windows
Component Shown
Frame is Visible with ExtendedState as ICONIFIED
Component Moved
Component Resized
Frame is Visible with ExtendedState as NORMAL
On MAC
Component Moved
Component Resized
Component Shown
Frame is Visible with ExtendedState as ICONIFIED
Frame is Visible with ExtendedState as NORMAL
There was a problem hiding this comment.
when form Frame.ICONIFIED to Frame.NORMAL componentMoved and componentResized events are triggered
Then verify that these events are triggered:
- if (componentShown || componentHidden) {
+ if (componentShown || componentHidden || !componentMoved
+ || !componentResized) {Otherwise it looks as if you forgot to test something. You may have two if statements to print more precise error message.
There was a problem hiding this comment.
We observed different behavior between MS Windows and other OS ,we natively received WM_SIZE/WM_MOVE events when set frame iconify to
deiconify , The AWT sends the events to components when it receives the events from the native system.
Now we are applying the OS specific check to test this scenario.
Please check the details at JDK-6754618
|
@ravigupta30 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
any volunteer for review? |
|
moved test to java/awt/Component |
|
I ran CI job with the latest code it passes without failure. |
|
/integrate |
|
@ravigupta30 |
|
/sponsor |
|
Going to push as commit a02d895.
Your commit was automatically rebased without conflicts. |
openjdk
bot
removed
ready
|
@aivanov-jdk @ravigupta30 Pushed as commit a02d895. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
This testcase checks for the following assertions for Component events:
Testing:
Tested using Mach5(20 times per platform) in macos,linux and windows and got all pass.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19521/head:pull/19521$ git checkout pull/19521Update a local copy of the PR:
$ git checkout pull/19521$ git pull https://git.openjdk.org/jdk.git pull/19521/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 19521View PR using the GUI difftool:
$ git pr show -t 19521Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19521.diff
Webrev
Link to Webrev Comment