8351372: Improve negative tests coverage of jpackage #23936

alexeysemenyukoracle · 2025-03-06T23:30:09Z

Changes to tests:

Added more test cases to ErrorTest.
Added functionality to jpackage test lib to facilitate new ErrorTest test cases.
Make all other negative tests use the CannedFormattedString class to validate error messages in jpackage output strictly.
Removed redundant negative tests/test cases duplicating ErrorTest.
Give unique descriptions to macos signing tests.

Changes to jpackage:

Main.java: Catch java.nio.file.NoSuchFileException as jdk.internal.opt.CommandLine.parse() throws this exception instead of java.io.FileNotFoundException to indicate missing command file. The issue was uncovered with the new ErrorTest test case for ERR_CannotParseOptions error ID.
DeployParams.java: replace ERR_MissingArgument with error.no-input-parameter as the error is not missing argument of --input parameter, but missing --input parameter itself. The issue was uncovered with the new ErrorTest test cases for ERR_MissingArgument and ``error.no-input-parameter` error IDs.
LauncherData.java: remove dead code as missing --input parameter is handled earlier in DeployParams.
DottedVersion.java: fix exception messages based on the new test cases in ErrorTest.
TokenRepalce.java/TokenRepalceTest.java: picked from 8341641: Make %APPDATA% and %LOCALAPPDATA% env variables available in *.cfg files #23923 PR. It is needed for ErrorTest as well.

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

Issues

JDK-8351372: Improve negative tests coverage of jpackage (Enhancement - P3)
JDK-8351446: Improve negative tests coverage of jpackage (CSR) (Withdrawn)

Reviewers

Alexander Matveev (@sashamatveev - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23936

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

Using diff file

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

Using Webrev

Link to Webrev Comment

…ning tests if multiple tests have the same description

…gBundle.cannedFormattedString()

…rmattedString.CannedArgument interface to make CannedFormattedString instances provide system-independent test descriptions.

…t --name option but with --runtime-image option. Allow to use arguments with CannedFormattedString that get their values from JPackageCommand instance. Make JPackageCommand.validateOutput(CannedFormattedString) more robust and allow it to take an array of canned formatted strings.

bridgekeeper · 2025-03-06T23:30:37Z

👋 Welcome back asemenyuk! 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 · 2025-03-06T23:31:34Z

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

8351372: Improve negative tests coverage of jpackage

Reviewed-by: almatvee

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

8d8bd0c: 8349492: Update sun/security/pkcs12/KeytoolOpensslInteropTest.java to use a recent Openssl version
73465b9: 8160327: Support for thumbnails present in APP1 marker for JPEG
dbdbbd4: 8348597: Update HarfBuzz to 10.4.0
7999091: 8351555: Help section added in JDK-8350638 uses invalid HTML
8450ae9: 8351440: Link with -reproducible on macOS
b40be22: 8333393: PhaseCFG::insert_anti_dependences can fail to raise LCAs and to add necessary anti-dependence edges
6b84bde: 8350007: Add usage message to the javadoc executable
32f2c2d: 8351017: ChronoUnit.MONTHS.between() not giving correct result when date is in February
d90b79a: 8351046: Rename ObjectMonitor functions
e90b6bd: 8350638: Make keyboard navigation more usable in API docs
... and 45 more: https://git.openjdk.org/jdk/compare/caaf4098452476d981183ad4302b76b9c883a72b...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 · 2025-03-06T23:32:03Z

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

core-libs

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.

…ype(); Add PackageType.isEnabled(). Make PackageType.isEnabled() and PackageType.isSupported() public.

…non-empty directory

…r tests covering `--runtime-image` option and the negative test duplicates a test in ErrorTest (the one covering ERR_MacAppStoreRuntimeBinExists)

…rTest test cases: '--mac-sign option is required' -> 'error.app-image.mac-sign.required'; 'Option [--mac-app-store] is not valid' -> 'ERR_InvalidOptionWithAppImageSigning'.

…mmands(). It duplicates `ERR_MissingJLinkOptMacAppStore` test case in ErrorTest

…t and is less strict on analyzing error output

…e runtime is used

…ame from the runtime image directory name if `--name` option is not given

… option.

alexeysemenyukoracle · 2025-03-07T19:52:21Z

@sashamatveev PTAL

mlbridge · 2025-03-07T19:55:09Z

Webrevs

alexeysemenyukoracle · 2025-03-08T00:59:27Z

Filed the CSR by accident.

sashamatveev

Looks good with minor comments.

sashamatveev · 2025-03-08T04:01:41Z

test/jdk/tools/jpackage/macosx/base/SigningBase.java

@@ -32,9 +32,23 @@

 public class SigningBase {

+    enum CertIndex {
+        ASCII_INDEX(0),
+        UNICODE_INDEX(0),


This one should be 1. Looks like we had a bug.

sashamatveev · 2025-03-08T04:18:13Z

test/jdk/tools/jpackage/share/ErrorTest.java

-                    JPackageStringBundle.MAIN.cannedFormattedString("ERR_MissingArgument", "--input")},
+            testSpec().addArgs("--type", "invalid-type")
+                    .error("ERR_InvalidInstallerType", "invalid-type"),
+            // no --input for non-mular app


non-mular -> non-modular

alexeysemenyukoracle · 2025-03-17T18:37:36Z

/integrate

openjdk · 2025-03-17T18:38:31Z

Going to push as commit 3aa6d62.
Since your change was applied there have been 142 commits pushed to the master branch:

3239919: 8350835: C2 SuperWord: assert/wrong result when using Float.float16ToFloat with byte instead of short input
47c1960: 8351689: -Xshare:dump with default classlist fails on static JDK
6b82b42: 8348598: Update Libpng to 1.6.47
2674a31: 8351891: Disable TestBreakSignalThreadDump.java#with_jsig and XCheckJSig.java on static JDK
4c6a523: 8352096: Test jdk/jfr/event/profiling/TestFullStackTrace.java shouldn't be executed with -XX:+DeoptimizeALot
d68775d: 8351995: JFR: Leftovers from removal of Security Manager
e62becc: 8350964: Add an ArtifactResolver.fetch(clazz) method
dbf47d6: 8351876: RISC-V: enable and fix some float round tests
d207ed3: 8352066: JVM.commit() and JVM.flush() exhibit race conditions against JFR epochs
0450ba9: 8351999: JFR: Incorrect scaling of throttled values
... and 132 more: https://git.openjdk.org/jdk/compare/caaf4098452476d981183ad4302b76b9c883a72b...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-03-17T18:38:36Z

@alexeysemenyukoracle Pushed as commit 3aa6d62.

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

alexeysemenyukoracle added 7 commits March 6, 2025 16:55

TestInstance: escape control characters in test description. Fail run…

1404e6d

…ning tests if multiple tests have the same description

More robust msi unpack when wix4 is used

c50fc5c

Can run launcher verifier with altered environment variables

08a2a66

Add Executor.setEnvVar()

ce854a6

Relax constraints on arguments for string formatting in JPackageStrin…

2ede257

…gBundle.cannedFormattedString()

Make CannedFormattedString a record and add null checks. Add CannedFo…

e74c14c

…rmattedString.CannedArgument interface to make CannedFormattedString instances provide system-independent test descriptions.

openjdk bot added the core-libs [email protected] label Mar 6, 2025

alexeysemenyukoracle added 5 commits March 6, 2025 22:23

Create fake runtime in unique directory

f2cd651

Support multiple output validators in JPackageCommand

4fc37b2

Make PackageType.getName() public and rename it into PackageType.getT…

185e38a

…ype(); Add PackageType.isEnabled(). Make PackageType.isEnabled() and PackageType.isSupported() public.

Move token substitution in generic TokenReplace class.

2838a08

More negative tests

9398463

alexeysemenyukoracle force-pushed the better-error-coverage-2 branch from d3a3551 to 67cbfb0 Compare March 7, 2025 03:24

alexeysemenyukoracle added 13 commits March 6, 2025 23:19

Better testing of DottedVersion: verify exception error messages

2706c90

Better test coverage for bad app image

5197f61

Test error reported by jpackage when --temp arg points to existing …

a4288bb

…non-empty directory

Remove MacAppStoreRuntimeTest.java. The positive test duplicates othe…

fefdf4d

…r tests covering `--runtime-image` option and the negative test duplicates a test in ErrorTest (the one covering ERR_MacAppStoreRuntimeBinExists)

Remove redundant PredefinedAppImageErrorTest.java. It duplicates Erro…

d85d67f

…rTest test cases: '--mac-sign option is required' -> 'error.app-image.mac-sign.required'; 'Option [--mac-app-store] is not valid' -> 'ERR_InvalidOptionWithAppImageSigning'.

Remove redundant MacAppStoreJlinkOptionsTest.testWithoutStripNativeCo…

915a15c

…mmands(). It duplicates `ERR_MissingJLinkOptMacAppStore` test case in ErrorTest

Better negative testing in MainClassTest

2aa3de5

NonExistentTest is redundant. It duplicates functionality of ErrorTes…

0c32b27

…t and is less strict on analyzing error output

ModulePathTest: use CannedFormattedString to validate error output

810aac3

FileAssociationsTest: use CannedFormattedString

30d42dc

Make JavaOptionsEqualsTest and JavaOptionsTest work when external fak…

3810b57

…e runtime is used

LinuxResourceTest: use CannedFormattedString

e853e56

Enhance RuntimePackageTest to test that jpackage can derive package n…

cc03a14

…ame from the runtime image directory name if `--name` option is not given

alexeysemenyukoracle added 2 commits March 7, 2025 00:20

Update copyright year

8e2db23

Ran ./bin/blessed-modifier-order.sh

dc5974f

alexeysemenyukoracle force-pushed the better-error-coverage-2 branch from 67cbfb0 to dc5974f Compare March 7, 2025 05:24

alexeysemenyukoracle added 3 commits March 7, 2025 00:37

Ignore default runtime if jpackage command line has --jlink-options…

8d72812

… option.

Give unique descriptions to signing tests

3fe3eed

Update copyright year

eda0efe

alexeysemenyukoracle marked this pull request as ready for review March 7, 2025 19:52

openjdk bot added the rfr Pull request is ready for review label Mar 7, 2025

openjdk bot added csr Pull request needs approved CSR before integration and removed csr Pull request needs approved CSR before integration labels Mar 7, 2025

sashamatveev reviewed Mar 8, 2025

View reviewed changes

alexeysemenyukoracle added 2 commits March 10, 2025 11:13

Fix Unicode signing certificate index

e6ad500

Fix typo

5458514

sashamatveev approved these changes Mar 10, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 10, 2025

Use Executor.trace() instead of TKit.trace()

3fdf653

openjdk bot removed the ready Pull request is ready to be integrated label Mar 10, 2025

sashamatveev approved these changes Mar 10, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 10, 2025

openjdk bot added the integrated Pull request has been integrated label Mar 17, 2025

openjdk bot closed this Mar 17, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 17, 2025

alexeysemenyukoracle mentioned this pull request Mar 18, 2025

8352275: Clean up dead code in jpackage revealed with improved negative test coverage #24099

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8351372: Improve negative tests coverage of jpackage #23936

8351372: Improve negative tests coverage of jpackage #23936

alexeysemenyukoracle commented Mar 6, 2025 •

edited by openjdk bot

Loading

bridgekeeper bot commented Mar 6, 2025

openjdk bot commented Mar 6, 2025 •

edited

Loading

openjdk bot commented Mar 6, 2025

alexeysemenyukoracle commented Mar 7, 2025

mlbridge bot commented Mar 7, 2025 •

edited

Loading

alexeysemenyukoracle commented Mar 8, 2025

sashamatveev left a comment

sashamatveev Mar 8, 2025

alexeysemenyukoracle Mar 10, 2025

sashamatveev Mar 8, 2025

alexeysemenyukoracle commented Mar 17, 2025

openjdk bot commented Mar 17, 2025

openjdk bot commented Mar 17, 2025

8351372: Improve negative tests coverage of jpackage #23936

8351372: Improve negative tests coverage of jpackage #23936

Conversation

alexeysemenyukoracle commented Mar 6, 2025 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Reviewing

bridgekeeper bot commented Mar 6, 2025

openjdk bot commented Mar 6, 2025 • edited Loading

openjdk bot commented Mar 6, 2025

alexeysemenyukoracle commented Mar 7, 2025

mlbridge bot commented Mar 7, 2025 • edited Loading

Webrevs

alexeysemenyukoracle commented Mar 8, 2025

sashamatveev left a comment

Choose a reason for hiding this comment

sashamatveev Mar 8, 2025

Choose a reason for hiding this comment

alexeysemenyukoracle Mar 10, 2025

Choose a reason for hiding this comment

sashamatveev Mar 8, 2025

Choose a reason for hiding this comment

alexeysemenyukoracle commented Mar 17, 2025

openjdk bot commented Mar 17, 2025

openjdk bot commented Mar 17, 2025

alexeysemenyukoracle commented Mar 6, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Mar 6, 2025 •

edited

Loading

mlbridge bot commented Mar 7, 2025 •

edited

Loading