-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8158801: [TEST_BUG] Mixing tests fail because of focus workaround trick #25971
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 kboulanou! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
@kboulanou 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
|
What do you mean by "focus workaround trick" and " minimizing area in the window control"? Are you seeing click is happening on the minimize button on the frame and frame getting iconified? |
In Windows machines, the click lands on the reduce control of the window, and this causes the windows to be iconified and the test fails. I have tested with undecorated frames, but the test continues to fail. The test needs to click on the middle of the Windows title bar (Outside of the controls) to pass. I will set the frames (The background undecorated frame, and the heavyweight component frames) on center as you have suggested. |
WHich test in particular? or is it all tests have this behavior?
I tried JMenuBarOverlapping.java but I dont see any click on title bar..The cursor in on top-left of "Test Menu" menu.. |
Minor: could you please update the copyright years? |
@prsadhuk I have reviewed the code again and I can point out where we do have the workaround click. It can be found in class SimpleOverlappingTestBase, in method performTest. Following is an excerpt of the source code:
I will use requestFocus as you have suggested and see if I get tests to pass. |
I will update copyright years for changed classes. |
@prsadhuk after applying your suggested change, I can see that it has fixed the issue for most of the tests but still the following tests didn't pass on Windows compared with the original fix of changing frame width : java/awt/Mixing/AWT_Mixing/JProgressBarInGlassPaneOverlapping.java I will check why these tests had failed. |
@kboulanou Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information. |
@prsadhuk All tests passed after the applying both changes: Awaiting time set to one second and checking if component has focus first before requesting focus. |
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.
Strangely enough, these changes make no difference for me when I run Mixing tests locally. The same number of tests fails with and without this fix. However, it seems to affect the CI.
Additionally, I still see the windows / frames are minimised on Windows. I don't think this is expected, is it? Thus, your previous suggestion for increasing the size of the windows looks reasonable to me.
test/jdk/java/awt/Mixing/AWT_Mixing/GlassPaneOverlappingTestBase.java
Outdated
Show resolved
Hide resolved
test/jdk/java/awt/Mixing/AWT_Mixing/GlassPaneOverlappingTestBase.java
Outdated
Show resolved
Hide resolved
test/jdk/java/awt/Mixing/AWT_Mixing/GlassPaneOverlappingTestBase.java
Outdated
Show resolved
Hide resolved
…se.java Co-authored-by: Alexey Ivanov <alexey.ivanov@oracle.com>
…se.java Co-authored-by: Alexey Ivanov <alexey.ivanov@oracle.com>
…se.java Co-authored-by: Alexey Ivanov <alexey.ivanov@oracle.com>
@aivanov-jdk both increasing the width of the frames and the requesting focus in frames worked for me. The test creates many frames and once a test completed for an AWT component, the next frame is created and shown on top of previously created frame (Maybe we can improve this by disposing the frame that finished testing ...). For each AWT component, a mixing frame is created. I will try on Windows and see if I can catch anything. |
@aivanov-jdk I have tested all tests with the new modifications and they passed. |
@aivanov-jdk I have tested on a Windows 11 Pro, before and after the fix in this PR, and I can see only 3 tests failing after an initial 26 failures. |
@prsadhuk I have updated this PR to center all possible tests in the middle of the screen. |
Centered some missed frames. Tested all de-problem listed tests on all platforms and tests passed. |
@@ -64,6 +64,7 @@ protected void prepareControls() { | |||
frame = new JFrame("Mixing : Dropdown Overlapping test"); | |||
frame.getContentPane().setLayout(new BoxLayout(frame.getContentPane(), BoxLayout.Y_AXIS)); | |||
frame.setSize(200, 200); | |||
frame.setLocationRelativeTo(null); |
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.
Are we disposing this frame somewhere(it's applicable to all the test files)?
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.
@manukumarvs in the original test, frames are not disposed. I will include disposing frame after testing the heavyweight component.
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.
It seems there are two types of testing, some use one frame (GlassPane) and others uses multiple frames. I have updated code to dispose of multiple frames. Currently testing my changes.
Many Mixing tests failed because the work around click lands on the minimizing area in the window control and causes the tests to fail.
This fix changes the width of base frames which allows most of tests to pass.
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25971/head:pull/25971
$ git checkout pull/25971
Update a local copy of the PR:
$ git checkout pull/25971
$ git pull https://git.openjdk.org/jdk.git pull/25971/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 25971
View PR using the GUI difftool:
$ git pr show -t 25971
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25971.diff
Using Webrev
Link to Webrev Comment