Skip to content
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

8163367: Test javax/swing/JComboBox/8033069/bug8033069NoScrollBar.java javax/swing/JComboBox/8033069/bug8033069ScrollBar.java fails intermittently #3678

Closed
wants to merge 4 commits into from

Conversation

@pankaj-bansal
Copy link

@pankaj-bansal pankaj-bansal commented Apr 25, 2021

The tests a javax/swing/JComboBox/8033069/bug8033069ScrollBar.java and javax/swing/JComboBox/8033069/bug8033069NoScrollBar.java fails intermittently. The tests have a few problems which could cause this behaviour and the are being fixed here.

  1. Test access the Swing components without EDT thread at couple of places.
  2. Test is not calling waitForIdle or delay after Robot.mouseMove operations.

There is some additional cleanup done in the fix and moved the frame in centre. The tests pass with multiple iterations on CI. Link in the JBS.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8163367: Test javax/swing/JComboBox/8033069/bug8033069NoScrollBar.java javax/swing/JComboBox/8033069/bug8033069ScrollBar.java fails intermittently

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/3678/head:pull/3678
$ git checkout pull/3678

Update a local copy of the PR:
$ git checkout pull/3678
$ git pull https://git.openjdk.java.net/jdk pull/3678/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3678

View PR using the GUI difftool:
$ git pr show -t 3678

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/3678.diff

…a javax/swing/JComboBox/8033069/bug8033069ScrollBar.java fails intermittently
@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Apr 25, 2021

👋 Welcome back pbansal! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

@openjdk openjdk bot commented Apr 25, 2021

@pankaj-bansal The following label will be automatically applied to this pull request:

  • swing

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.

@openjdk openjdk bot added the swing label Apr 25, 2021
@pankaj-bansal pankaj-bansal marked this pull request as ready for review Apr 25, 2021
@openjdk openjdk bot added the rfr label Apr 25, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Apr 25, 2021

Webrevs

@mrserb
Copy link
Member

@mrserb mrserb commented Apr 25, 2021

@aivanov-jdk please take a look

private Point p;
private Dimension d;
Copy link
Member

@aivanov-jdk aivanov-jdk Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both should be volatile as they're accessed from two different threads.

p = cb1.getLocationOnScreen();
d = cb1.getSize();
});
robot.waitForIdle();
Copy link
Member

@aivanov-jdk aivanov-jdk Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it needed here?
The block above does not modify the state of the UI.

robot.mouseMove(p.x + d.width / 2, p.y + d.height / 2);
robot.waitForIdle();
Copy link
Member

@aivanov-jdk aivanov-jdk Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the delay between mouseMove and mousePress/Release is unneeded, does it really make a difference?

if (frame != null) {
frame.dispose();
SwingUtilities.invokeAndWait(frame::dispose);
}
Copy link
Member

@aivanov-jdk aivanov-jdk Apr 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The frame is still accessed from two different threads.

Review comments

Co-authored-by: Alexey Ivanov <70774172+aivanov-jdk@users.noreply.github.com>
@openjdk
Copy link

@openjdk openjdk bot commented Apr 27, 2021

@pankaj-bansal 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:

8163367: Test javax/swing/JComboBox/8033069/bug8033069NoScrollBar.java javax/swing/JComboBox/8033069/bug8033069ScrollBar.java fails intermittently

Reviewed-by: aivanov

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 199 new commits pushed to the master branch:

  • f6e26f6: 8265756: AArch64: initialize memory allocated for locals according to Windows AArch64 stack page growth requirement in template interpreter
  • 0a4c338: 8263432: javac may report an invalid package/class clash on case insensitive filesystems
  • 82b3719: 8265967: Unused NullCheckNode forward declaration in node.hpp
  • 468c847: 8234020: Remove FullGCCount_lock
  • 4785e11: 8264806: Remove the experimental JIT compiler
  • 7db9330: 8196300: java/awt/TextArea/TextAreaScrolling/TextAreaScrolling.java times out
  • 694aced: 8264805: Remove the experimental Ahead-of-Time Compiler
  • 15d4787: 8265496: improve null check in DeflaterOutputStream/InflaterInputStream
  • 1d37b98: 8265972: Remove declarations with no implementations in javaClasses.hpp
  • cb29b19: 8184134: HeapRegion::LogOfHRGrainWords is unused
  • ... and 189 more: https://git.openjdk.java.net/jdk/compare/81877f7df20c29e6716a86a1dac3eb0868d768bf...master

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 master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready label Apr 27, 2021
@pankaj-bansal
Copy link
Author

@pankaj-bansal pankaj-bansal commented Apr 27, 2021

/integrate

@openjdk openjdk bot closed this Apr 27, 2021
@openjdk openjdk bot added integrated and removed ready rfr labels Apr 27, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Apr 27, 2021

@pankaj-bansal Since your change was applied there have been 202 commits pushed to the master branch:

  • 9adbf15: 8265995: Shenandoah: Move ShenandoahInitMarkRootsClosure close to its use
  • 879a77f: 8265757: stack-use-after-scope in perfMemory_posix.cpp get_user_name_slow()
  • e4be968: 8265980: Fix systemDictionary and loaderConstraints printing
  • f6e26f6: 8265756: AArch64: initialize memory allocated for locals according to Windows AArch64 stack page growth requirement in template interpreter
  • 0a4c338: 8263432: javac may report an invalid package/class clash on case insensitive filesystems
  • 82b3719: 8265967: Unused NullCheckNode forward declaration in node.hpp
  • 468c847: 8234020: Remove FullGCCount_lock
  • 4785e11: 8264806: Remove the experimental JIT compiler
  • 7db9330: 8196300: java/awt/TextArea/TextAreaScrolling/TextAreaScrolling.java times out
  • 694aced: 8264805: Remove the experimental Ahead-of-Time Compiler
  • ... and 192 more: https://git.openjdk.java.net/jdk/compare/81877f7df20c29e6716a86a1dac3eb0868d768bf...master

Your commit was automatically rebased without conflicts.

Pushed as commit 9481fad.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@pankaj-bansal pankaj-bansal deleted the JDK-8163367 branch Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants