JDK-8258884: [TEST_BUG] Convert applet-based test open/test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java to a regular java test #1878

skodanda · 2020-12-23T11:50:41Z

Hi All, Could you please review this fix for JDK16?

Problem Description: The test open/test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java is applet based.

Fix: Rewritten the above applet based test to a regular java test.

Best Regards,
K Suman Rajkumaar

Progress

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

Issue

JDK-8258884: [TEST_BUG] Convert applet-based test open/test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java to a regular java test

Reviewers

Alexey Ivanov (@aivanov-jdk - Reviewer) ⚠️ Review applies to af8de10
Sergey Bylokhov (@mrserb - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1878/head:pull/1878
$ git checkout pull/1878

bridgekeeper · 2020-12-23T11:51:09Z

👋 Welcome back skodanda! 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 · 2020-12-23T11:52:32Z

@skodanda 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.

mlbridge · 2020-12-23T11:56:02Z

Webrevs

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java

@run

Corrected the jtreg tag from @run main bug8031573 to @run main/manual bug8031573 to make this test as a manual test.

aivanov-jdk · 2021-01-05T08:35:08Z

You should remove the HTML which hosts the applet.

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java

aivanov-jdk · 2021-01-05T08:28:34Z

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java

+            passed = false;
+            latch.countDown();
+            frame.dispose();
+            throw new RuntimeException("Test Failed!");


Throwing the exception on EDT is redundant and does not make the test fail; moreover you throw the exception in main() if passed is false.

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java

Updated with another set of review comments from Alexey.

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java

openjdk · 2021-01-06T19:53:20Z

@skodanda 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:

8258884: [TEST_BUG] Convert applet-based test open/test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java to a regular java test

Reviewed-by: aivanov, serb

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

2c8e337: 8259622: TreeMap.computeIfAbsent deviates from spec
d701bab: Merge
4307fa6: 8253505: JFR: onFlush invoked out of order with a sorted event stream
0148adf: 8255120: C2: assert(outer->outcnt() >= phis + 2 && outer->outcnt() <= phis + 2 + stores + 1) failed: only phis
90960c5: 8252657: JVMTI agent is not unloaded when Agent_OnAttach is failed
e3b548a: 8257736: InputStream from BodyPublishers.ofInputStream() leaks when IOE happens
978bed6: 8259522: Apply java.io.Serial annotations in java.desktop
bf28f92: 8259713: Fix comments about ResetNoHandleMark in deoptimization
4f881ba: 8258652: Assert in JvmtiThreadState::cur_stack_depth() can noticeably slow down debugging single stepping
d18d26e: 8259350: Add some internal debugging APIs to the debug agent
... and 642 more: https://git.openjdk.java.net/jdk/compare/f2f3ba92d78da6af3bf7e379cad436bdfa3d29bc...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.

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 (@aivanov-jdk, @mrserb) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

prrace · 2021-01-06T20:42:55Z

"Hi All, Could you please review this fix for JDK16?"
This is for JDK 17, not JDK 16, right ? I can't imagine why it would be for 16 at this juncture ..

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java

aivanov-jdk · 2021-01-07T15:54:12Z

"Hi All, Could you please review this fix for JDK16?"
This is for JDK 17, not JDK 16, right ? I can't imagine why it would be for 16 at this juncture ..

No, it's not meant for JDK 16 — it's for jdk-dev, the main development line which is JDK 17 at this very moment.

skodanda · 2021-01-12T10:25:28Z

/review-requested

skodanda · 2021-01-13T16:27:24Z

Dear Phil/Sergey,

I have taken care of the review comments. Please have a look.

Best Regards,
K Suman Rajkumaar

skodanda · 2021-01-15T05:58:00Z

/integrate

openjdk · 2021-01-15T05:58:37Z

@skodanda
Your change (at version b51bf3b) is now ready to be sponsored by a Committer.

aivanov-jdk · 2021-01-15T09:39:50Z

/sponsor

openjdk · 2021-01-15T09:40:48Z

@aivanov-jdk @skodanda Since your change was applied there have been 653 commits pushed to the master branch:

6d4a593: 8259627: Potential memory leaks in JVMTI after JDK-8227745
2c8e337: 8259622: TreeMap.computeIfAbsent deviates from spec
d701bab: Merge
4307fa6: 8253505: JFR: onFlush invoked out of order with a sorted event stream
0148adf: 8255120: C2: assert(outer->outcnt() >= phis + 2 && outer->outcnt() <= phis + 2 + stores + 1) failed: only phis
90960c5: 8252657: JVMTI agent is not unloaded when Agent_OnAttach is failed
e3b548a: 8257736: InputStream from BodyPublishers.ofInputStream() leaks when IOE happens
978bed6: 8259522: Apply java.io.Serial annotations in java.desktop
bf28f92: 8259713: Fix comments about ResetNoHandleMark in deoptimization
4f881ba: 8258652: Assert in JvmtiThreadState::cur_stack_depth() can noticeably slow down debugging single stepping
... and 643 more: https://git.openjdk.java.net/jdk/compare/f2f3ba92d78da6af3bf7e379cad436bdfa3d29bc...master

Your commit was automatically rebased without conflicts.

Pushed as commit b01a15e.

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

Rewritten applet based test as a regular java test app.

9dbc094

openjdk bot added the rfr Pull request is ready for review label Dec 23, 2020

openjdk bot added the swing client-libs-dev@openjdk.org label Dec 23, 2020

mrserb reviewed Dec 31, 2020

View reviewed changes

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java Outdated Show resolved Hide resolved

Update bug8031573.java

5f1ab6e

Corrected the jtreg tag from @run main bug8031573 to @run main/manual bug8031573 to make this test as a manual test.

aivanov-jdk suggested changes Jan 5, 2021

View reviewed changes

skodanda added 4 commits January 6, 2021 13:45

Incorporated the review comments from Alexey.

385a6ce

Update bug8031573.java

d51d774

Update bug8031573.java

9020f7b

Deleted the bug8031573.html

0e675a0

aivanov-jdk suggested changes Jan 6, 2021

View reviewed changes

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java Outdated Show resolved Hide resolved

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java Outdated Show resolved Hide resolved

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java Outdated Show resolved Hide resolved

Updated with another set of review comments from Alexey

72c48ff

Updated with another set of review comments from Alexey.

aivanov-jdk suggested changes Jan 6, 2021

View reviewed changes

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java Outdated Show resolved Hide resolved

Removed the setSize()

af8de10

aivanov-jdk approved these changes Jan 6, 2021

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 6, 2021

prrace reviewed Jan 6, 2021

View reviewed changes

test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java Show resolved Hide resolved

Changed "used" to "used for" as per Phil's review comment.

b51bf3b

mrserb approved these changes Jan 13, 2021

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Jan 15, 2021

openjdk bot closed this Jan 15, 2021

openjdk bot added integrated Pull request has been integrated and removed sponsor Pull request is ready to be sponsored ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 15, 2021

JDK-8258884: [TEST_BUG] Convert applet-based test open/test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java to a regular java test #1878

JDK-8258884: [TEST_BUG] Convert applet-based test open/test/jdk/javax/swing/JMenuItem/8031573/bug8031573.java to a regular java test #1878

Uh oh!

Conversation

skodanda commented Dec 23, 2020 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Download

Uh oh!

bridgekeeper bot commented Dec 23, 2020

Uh oh!

openjdk bot commented Dec 23, 2020

Uh oh!

mlbridge bot commented Dec 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Uh oh!

aivanov-jdk commented Jan 5, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aivanov-jdk Jan 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openjdk bot commented Jan 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prrace commented Jan 6, 2021

Uh oh!

Uh oh!

aivanov-jdk commented Jan 7, 2021

Uh oh!

skodanda commented Jan 12, 2021

Uh oh!

skodanda commented Jan 13, 2021

Uh oh!

skodanda commented Jan 15, 2021

Uh oh!

openjdk bot commented Jan 15, 2021

Uh oh!

aivanov-jdk commented Jan 15, 2021

Uh oh!

openjdk bot commented Jan 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

skodanda commented Dec 23, 2020 •

edited by openjdk bot

Loading

mlbridge bot commented Dec 23, 2020 •

edited

Loading

openjdk bot commented Jan 6, 2021 •

edited

Loading