8342602: Remove JButton/PressedButtonRightClickTest test

[aivanov-jdk]

The javax/swing/JButton/PressedButtonRightClickTest.java test that was added by JDK-8049069 fully duplicates an existing test javax/swing/JButton/bug4490179.java.

To save time running tests, I'm removing PressedButtonRightClickTest.java.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8342602: Remove JButton/PressedButtonRightClickTest test (Bug - P4)

Reviewers

• Damon Nguyen (@DamonGuy - Committer)
• Phil Race (@prrace - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21587/head:pull/21587
$ git checkout pull/21587

Update a local copy of the PR:
$ git checkout pull/21587
$ git pull https://git.openjdk.org/jdk.git pull/21587/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 21587

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21587.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back aivanov! 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]

@aivanov-jdk 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:

8342602: Remove JButton/PressedButtonRightClickTest test

Reviewed-by: dnguyen, prr

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

• 087a07b: 8343479: Remove unnecessary @SuppressWarnings annotations (hotspot)
• 02ec8ca: 8342508: Use latch in BasicMenuUI/bug4983388.java instead of delay
• 47ebf8d: 8342098: Write a test to compare the images
• 78602be: 8344024: Unnecessary Hashtable usage in RSAPSSSignature.DIGEST_LENGTHS
• 7f672eb: 8344550: Compilation error of jpackage test JPackageStringBundle.java source
• 48223f7: 8344143: Test jdk/java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java timed out on macosx-x64
• d4cd27e: 8344445: MethodCounters don't need a vptr
• dc940ec: 8344387: RISC-V: C2: Improve encoding of LoadNKlass for compact headers
• ded01e4: 8344327: SM cleanup in jdk.unsupported ReflectionFactory
• 59fcfae: 8343129: Disable unstable check of ThreadsListHandle.sanity_vm ThreadList values
• ... and 244 more: https://git.openjdk.org/jdk/compare/c0e6c3b93c0d21debc538e0135805c2957053108...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]

@aivanov-jdk The following label will be automatically applied to this pull request:

• client

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.

[mlbridge]

Webrevs

• 01: Full - Incremental (950cc8f2)
• 00: Full (bde089aa)

[mrserb]

The bug4490179 verifies the code using the listener added to the button while the PressedButtonRightClickTest verifies the code using getModel().isPressed() which is not necessary the same.

btw it is unclear why the bug4490179 test is not mention in the https://bugs.openjdk.org/browse/JDK-8049069

[aivanov-jdk]

The bug4490179 verifies the code using the listener added to the button while the PressedButtonRightClickTest verifies the code using getModel().isPressed() which is not necessary the same.

The description of JDK-8049069:

1. Press JButton with left mouse button and don't release it.
2. While holding left button, press and release right button
   Expected behaviour: JButton is still pressed
   Actual behaviour: JButton is released.

This matches the description of JDK-4490179:

On a JButton, press the left mouse button and, without releasing it, click the right button. This (incorrectly) fires an ActionEvent and disarms the button.

Indeed, if I add ActionListener to JButton to the test case JButtonReleaseBug.java attached to JDK-8049069, the action event is triggered as soon as I release the right mouse button because the model goes from pressed state to released state.

Even though the tests verify slightly different conditions, the effect is the same. The button must remain pressed when right mouse button is pressed and released.

btw it is unclear why the bug4490179 test is not mention in the https://bugs.openjdk.org/browse/JDK-8049069

I do not know. I assume the reporter of JDK-8049069 didn't know a test had already existed.

According the first comment, “Detected by test: Swing_JButton/Automated/RButtonEventOP.” It looks the bug was found by SQE and a test in the SQE test suite.

[mrserb]

Even though the tests verify slightly different conditions, the effect is the same. The button must remain pressed when right mouse button is pressed and released.

But it's still a different condition, and removing the test will reduce coverage.

[aivanov-jdk]

Even though the tests verify slightly different conditions, the effect is the same. The button must remain pressed when right mouse button is pressed and released.

But it's still a different condition, and removing the test will reduce coverage.

No, it will not reduce the coverage. If model.isPressed() returns true, it means the action event isn't triggered; if model.isPressed() becomes false, the action event is triggered.

Both tests verify the same end result: no action event is triggered if mouse button 3 is pressed and released while mouse button 1 is pressed. bug4490179.java does it explicitly whereas PressedButtonRightClickTest.java does it implicitly by checking the state of the button model.

[DamonGuy]

Even though the tests verify slightly different conditions, the effect is the same. The button must remain pressed when right mouse button is pressed and released.

But it's still a different condition, and removing the test will reduce coverage.

No, it will not reduce the coverage. If model.isPressed() returns true, it means the action event isn't triggered; if model.isPressed() becomes false, the action event is triggered.

Both tests verify the same end result: no action event is triggered if mouse button 3 is pressed and released while mouse button 1 is pressed. bug4490179.java does it explicitly whereas PressedButtonRightClickTest.java does it implicitly by checking the state of the button model.

Looking at both of these tests, it does look like the coverage will remain the same. Looks like there's just two ways to test the same functionality, and the original issue reported for each bug is virtually identical.

If removing one test is actually problematic, you could edit the remaining test to check for both isPressed() and for the action event trigger, but that seems unnecessary. And I think checking for the action event rather than the state of the button just makes more sense.

Removing the suggested test seems right to me. The additional test was probably just created due to not knowing the older test existed as mentioned.

[aivanov-jdk]

/integrate

[openjdk]

Going to push as commit 8647c00.
Since your change was applied there have been 526 commits pushed to the master branch:

• fcf185c: 8345325: SM cleanup of GetPropertyAction in java.base
• eac00f6: 8345396: Fix headers after JDK-8345164
• dfa5620: 8345164: Remove residual --enable-preview in FFM tests and benchmarks
• 65b5a2e: 8345158: IGV: local scheduling should not place successors before predecessors
• 8cad043: 8336768: Allow captureCallState and critical linker options to be combined
• 63af2f4: 8344414: ZGC: Another division by zero in rule_major_allocation_rate
• 077b842: 8345074: java.net.InterfaceAddress constructor could be made private
• ec93cc5: 8343932: Error when parsing qualified generic type test pattern in switch
• c330b90: 8343780: Add since checker tests to the Tools area modules and add missing @SInCE to jdk.jfr
• 8dada73: 8345120: A likely bug in StringSupport::chunkedStrlenShort
• ... and 516 more: https://git.openjdk.org/jdk/compare/c0e6c3b93c0d21debc538e0135805c2957053108...master

Your commit was automatically rebased without conflicts.

[openjdk]

@aivanov-jdk Pushed as commit 8647c00.

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