8344316: security/auth/callback/TextCallbackHandler/Password.java make runnable with JTReg and add the UI #22951

myankelev · 2025-01-07T16:19:58Z

Changed security/auth/callback/TextCallbackHandler/Password.java to run with JTReg, now the dialog box with instructions will appear as a ui

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-8344316: security/auth/callback/TextCallbackHandler/Password.java make runnable with JTReg and add the UI (Bug - P4)

Reviewers

Weijun Wang (@wangweij - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22951

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

Using diff file

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

Using Webrev

Link to Webrev Comment

… fail if run with JTReg test path security/auth/callback/TextCallbackHandler/Password.java

bridgekeeper · 2025-01-07T16:20:38Z

Hi @myankelev, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user myankelev" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

openjdk · 2025-01-07T16:20:55Z

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

8344316: security/auth/callback/TextCallbackHandler/Password.java make runnable with JTReg and add the UI

Reviewed-by: weijun

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

2f2f7cf: 8349084: Update vectors used in several PQC benchmarks
b9b62a0: 8346792: serviceability/jvmti/vthread/GetThreadState/GetThreadState.java testObjectWaitMillis failed
6b994cd: 8333697: C2: Hit MemLimit in PhaseCFG::global_code_motion
2ff8440: 8349344: Clarify documentation of Arena.ofConfined
19399d2: 8348572: C2 compilation asserts due to unexpected irreducible loop
6146588: 8348190: Framework for tracing makefile inclusion and parsing
66a3898: 8348659: AArch64: IR rule failure with compiler/loopopts/superword/TestSplitPacks.java
40603a5: 8349214: Improve size optimization flags for MSVC builds
0926949: 8347629: Test FailOverDirectExecutionControlTest.java fails with -Xcomp
a51e669: 8349200: [JMH] time.format.ZonedDateTimeFormatterBenchmark fails
... and 417 more: https://git.openjdk.org/jdk/compare/f1196638b347c9a2e7d7dc1795c6c24ea7efe711...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 (@wangweij) 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).

openjdk · 2025-01-07T16:21:28Z

@myankelev The following label will be automatically applied to this pull request:

security

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.

myankelev · 2025-01-07T16:43:35Z

/covered

bridgekeeper · 2025-01-07T16:44:31Z

Thank you! Please allow for a few business days to verify that your employer has signed the OCA. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated!

offamitkumar · 2025-01-08T06:40:15Z

test/jdk/com/sun/security/auth/callback/TextCallbackHandler/Password.java

@@ -45,6 +48,11 @@

 public class Password {
   public static void main(String args[]) throws Exception {
+
+        if (System.getProperty("java.class.path").contains("jtreg")){
+            throw new RuntimeException("This is a manual testing, it shouldn't be run with jtreg.");


Was a crash necessary ? Can't we bail out ?

I think it is good to put. As this the manual test and ran by IMT. They are raising issue if the test is failing and actually not going inside the test and checking the steps how to run. As they run all the test using jtreg they will just trigger in bunch. So if we give proper message that "this test will failing while running jtreg , please go and see the steps inside the test to run" will be good

Thank you, I'll leave it in order to raise an issue. I will also change the wording a bit, to fix the grammar a bit

Thanks looking good now.

offamitkumar · 2025-01-08T06:40:47Z

Also please enable GHA on your repo.

mlbridge · 2025-01-08T14:40:08Z

Webrevs

myankelev · 2025-01-10T12:21:18Z

Also please enable GHA on your repo.

It run once the OCA was confirmed

test/jdk/com/sun/security/auth/callback/TextCallbackHandler/Password.java

…enerated by jtreg started ui

test/jdk/com/sun/security/auth/callback/TextCallbackHandler/Password.java

wangweij · 2025-01-23T16:01:59Z

Since you have a different fix now, please update the bug title.

…r to jtreg anotation

…ructions change

myankelev · 2025-02-05T18:20:55Z

/integrate

openjdk · 2025-02-05T18:22:06Z

@myankelev
Your change (at version 56d1d3e) is now ready to be sponsored by a Committer.

wangweij · 2025-02-05T18:23:07Z

/sponsor

openjdk · 2025-02-05T18:23:28Z

Going to push as commit 82bc0a7.
Since your change was applied there have been 427 commits pushed to the master branch:

2f2f7cf: 8349084: Update vectors used in several PQC benchmarks
b9b62a0: 8346792: serviceability/jvmti/vthread/GetThreadState/GetThreadState.java testObjectWaitMillis failed
6b994cd: 8333697: C2: Hit MemLimit in PhaseCFG::global_code_motion
2ff8440: 8349344: Clarify documentation of Arena.ofConfined
19399d2: 8348572: C2 compilation asserts due to unexpected irreducible loop
6146588: 8348190: Framework for tracing makefile inclusion and parsing
66a3898: 8348659: AArch64: IR rule failure with compiler/loopopts/superword/TestSplitPacks.java
40603a5: 8349214: Improve size optimization flags for MSVC builds
0926949: 8347629: Test FailOverDirectExecutionControlTest.java fails with -Xcomp
a51e669: 8349200: [JMH] time.format.ZonedDateTimeFormatterBenchmark fails
... and 417 more: https://git.openjdk.org/jdk/compare/f1196638b347c9a2e7d7dc1795c6c24ea7efe711...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-02-05T18:23:38Z

@wangweij @myankelev Pushed as commit 82bc0a7.

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

myankelev added 2 commits January 7, 2025 16:03

8344316: Add comment to make clear it is not a JTReg test & test will…

fab7793

… fail if run with JTReg test path security/auth/callback/TextCallbackHandler/Password.java

format small change

c39d99a

bridgekeeper bot added the oca Needs verification of OCA signatory status label Jan 7, 2025

openjdk bot added the security security-dev@openjdk.org label Jan 7, 2025

bridgekeeper bot added the oca-verify Needs verification of OCA signatory status label Jan 7, 2025

offamitkumar reviewed Jan 8, 2025

View reviewed changes

bridgekeeper bot removed oca Needs verification of OCA signatory status oca-verify Needs verification of OCA signatory status labels Jan 8, 2025

openjdk bot added the rfr Pull request is ready for review label Jan 8, 2025

myankelev added 2 commits January 8, 2025 19:00

minor, but to the @bug

d887a60

chaned the wording of the error and removed test bug number

ffc2be1

removed an unnecessary new line

f162ccc

myankelev commented Jan 14, 2025

View reviewed changes

test/jdk/com/sun/security/auth/callback/TextCallbackHandler/Password.java Show resolved Hide resolved

myankelev added 3 commits January 21, 2025 18:09

reworked the test to be able to run automatically with jtreg

43553d6

changed the test back to manual. Command to start the script is now g…

1840d7a

…enerated by jtreg started ui

typos

b937cf2

wangweij reviewed Jan 23, 2025

View reviewed changes

test/jdk/com/sun/security/auth/callback/TextCallbackHandler/Password.java Outdated Show resolved Hide resolved

wangweij reviewed Jan 23, 2025

View reviewed changes

test/jdk/com/sun/security/auth/callback/TextCallbackHandler/Password.java Outdated Show resolved Hide resolved

Wording change, command is single line now and added othervm paramete…

1944e43

…r to jtreg anotation

myankelev changed the title ~~8344316: Add extra line in security/auth/callback/TextCallbackHandler/Password.java to make clear it is not a JTReg test~~ 8344316: security/auth/callback/TextCallbackHandler/Password.java make runnable with JTReg and add the UI Jan 24, 2025

visible and invisible order change, support for Cygwin and minor inst…

cbbe428

…ructions change

minor, spelling and instructions

56d1d3e

wangweij approved these changes Feb 5, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 5, 2025

openjdk bot added the sponsor Pull request is ready to be sponsored label Feb 5, 2025

openjdk bot added the integrated Pull request has been integrated label Feb 5, 2025

openjdk bot closed this Feb 5, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Feb 5, 2025

8344316: security/auth/callback/TextCallbackHandler/Password.java make runnable with JTReg and add the UI #22951

8344316: security/auth/callback/TextCallbackHandler/Password.java make runnable with JTReg and add the UI #22951

Uh oh!

Conversation

myankelev commented Jan 7, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jan 7, 2025

Uh oh!

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

Uh oh!

Uh oh!

openjdk bot commented Jan 7, 2025

Uh oh!

myankelev commented Jan 7, 2025

Uh oh!

bridgekeeper bot commented Jan 7, 2025

Uh oh!

offamitkumar Jan 8, 2025

Choose a reason for hiding this comment

Uh oh!

Shivangi-aa Jan 10, 2025

Choose a reason for hiding this comment

Uh oh!

myankelev Jan 10, 2025

Choose a reason for hiding this comment

Uh oh!

Shivangi-aa Jan 10, 2025

Choose a reason for hiding this comment

Uh oh!

offamitkumar commented Jan 8, 2025

Uh oh!

mlbridge bot commented Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

myankelev commented Jan 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wangweij commented Jan 23, 2025

Uh oh!

myankelev commented Feb 5, 2025

Uh oh!

openjdk bot commented Feb 5, 2025

Uh oh!

wangweij commented Feb 5, 2025

Uh oh!

openjdk bot commented Feb 5, 2025

Uh oh!

openjdk bot commented Feb 5, 2025

Uh oh!

Uh oh!

myankelev commented Jan 7, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jan 7, 2025 •

edited

Loading

mlbridge bot commented Jan 8, 2025 •

edited

Loading

myankelev commented Jan 10, 2025 •

edited

Loading