8349564: Clean warnings found in jpackage tests when building them with -Xlint:all #23455

alexeysemenyukoracle · 2025-02-05T04:14:21Z

Clean warnings found in jpackage tests when building them with -Xlint:all option in Eclipse IDE.

They are:

redundant imports (solution: remove)
unused function/fields (solution: remove)
missing SuppressWarnings-s (solution: add what is missing)
redundant SuppressWarnings-s (solution: remove)
raw types used (solution: use wildcard or more specific types if appropriate)
generic varargs (solution: convert to single/double/list arguments)
an incomplete list of enum elements in switch statements (solution: add default branch)

To prevent regression, added -Xlint:all -Werror to the compiler command line of all jpackage tests

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-8349564: Clean warnings found in jpackage tests when building them with -Xlint:all (Bug - P4)

Reviewers

Alexander Matveev (@sashamatveev - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 23455

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

Using diff file

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

Using Webrev

Link to Webrev Comment

…ompiling with `-Xlint:all` option

bridgekeeper · 2025-02-05T04:15:14Z

👋 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-02-05T04:16:12Z

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

8349564: Clean warnings found in jpackage tests when building them with -Xlint:all

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

7a0832f: 8349934: Wrong file regex for copyright header format check in .jcheck/conf
2a5d1da: 8349571: Remove JavaThreadFactory interface from SA
74e458b: 8349684: Remove SA core file tests from problem list for macosx-x64
ba28119: 8348427: DeferredLintHandler API should use JCTree instead of DiagnosticPosition
c5ac3c4: 8249831: Test sun/security/mscapi/nonUniqueAliases/NonUniqueAliases.java is marked with @ignore
336d0d8: 8349926: [BACKOUT] Support static JDK in libfontmanager/freetypeScaler.c
4b463ee: 8342103: C2 compiler support for Float16 type and associated scalar operations
332d87c: 8349859: Support static JDK in libfontmanager/freetypeScaler.c
73e1780: 8349836: G1: Improve group prediction log message
ed17c55: 8349145: Make Class.getProtectionDomain() non-native
... and 52 more: https://git.openjdk.org/jdk/compare/eb84702626ef1ededc55969923958e8c2835d821...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-02-05T04:16:51Z

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

openjdk · 2025-02-06T17:21:40Z

@alexeysemenyukoracle this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout jlint-tests
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

…age tests

mlbridge · 2025-02-06T18:12:05Z

Webrevs

sashamatveev · 2025-02-06T22:36:51Z

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TestBuilder.java

        });
    }

    // Wraps Method.invike() into ThrowingRunnable.run()


invike -> invoke.

sashamatveev · 2025-02-06T22:49:20Z

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

 * @library /test/jdk/tools/jpackage/helpers
 * @build jdk.jpackage.test.*
- * @compile JavaOptionsTest.java
+ * @compile -Xlint:all -Werror JavaOptionsTest.java


Can we add -Xlint:all -Werror in generic way instead of repeating each time?

I don't think so.
@sormuras does jtreg allow to keep javac args for a group of tests in one place?

sashamatveev · 2025-02-06T22:50:23Z

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

 * @library /test/jdk/tools/jpackage/helpers
 * @build jdk.jpackage.test.*


Helper classes will be build without -Xlint:all -Werror, right? If yes do we need to add -Xlint:all -Werror as well when building helper classes?

do we need to add -Xlint:all -Werror as well when building helper classes?

Good question. They are implicitly built by jtreg when requested through @build jdk.jpackage.test.* tag. I don't know how it constructs javac command line. @sormuras can you comment?

alexeysemenyukoracle · 2025-02-12T15:48:12Z

Current limitations of jtreg don't allow us to avoid repeating -Xlint:all -Werror in every test, and there is no way to specify javac options for library code. It can be revisited when jtreg gets corresponding features. @sashamatveev please finalize the review.

sashamatveev

Looks good.

alexeysemenyukoracle · 2025-02-13T10:20:59Z

/integrate

openjdk · 2025-02-13T10:22:24Z

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

29202d1: 8349648: Test tools/jpackage/share/JLinkOptionsTest.java fails with --enable-linkable-runtime set after JDK-8346434
24b7f81: 8349783: g1RemSetSummary.cpp:344:68: runtime error: member call on null pointer of type 'struct G1HeapRegion'
a637ccf: 8349851: RISC-V: Call VM leaf can use movptr2
adc3f53: 8349787: java/lang/Thread/virtual/ThreadPollOnYield.java#default passes unexpectedly without libVThreadPinner.so
55097dd: 8344802: Crash in StubRoutines::verify_mxcsr with -XX:+EnableX86ECoreOpts and -Xcheck:jni
b8576eb: 6562489: Font-Renderer should ignore invisible characters \u2062 and \u2063
7a0832f: 8349934: Wrong file regex for copyright header format check in .jcheck/conf
2a5d1da: 8349571: Remove JavaThreadFactory interface from SA
74e458b: 8349684: Remove SA core file tests from problem list for macosx-x64
ba28119: 8348427: DeferredLintHandler API should use JCTree instead of DiagnosticPosition
... and 58 more: https://git.openjdk.org/jdk/compare/eb84702626ef1ededc55969923958e8c2835d821...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-02-13T10:22:36Z

@alexeysemenyukoracle Pushed as commit efc597b.

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

alexeysemenyukoracle added 7 commits January 9, 2025 15:20

Fix warnings in jpackage code found by building it in Eclipse

1f9f14a

Fix warning found in jpackage tests by building then in Eclipse and c…

4d72be7

…ompiling with `-Xlint:all` option

Fix for tests

a0b5c2c

Merge branch 'master' into jlint-tests

b74dacd

"Cleanup" bugfix. Double facepalm

1b0cb97

Merge branch 'master' into jlint-tests

d255f19

More jlint errors fixed

410d738

openjdk bot added the core-libs core-libs-dev@openjdk.org label Feb 5, 2025

alexeysemenyukoracle added 2 commits February 4, 2025 23:29

Undo jlint clean up for the implementation

0e283bc

Copyright year updated

77cded3

alexeysemenyukoracle changed the title ~~Jlint tests~~ 8349564: Clean warnings found in jpackage tests when building them with -Xlint:all Feb 6, 2025

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Feb 6, 2025

alexeysemenyukoracle added 3 commits February 6, 2025 12:38

Merge branch 'master' into jlint-tests

259cffc

Bad merge fix

68d7d45

Add -Xlint:all and -Werror args to compiler command line of all jpack…

23044cf

…age tests

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Feb 6, 2025

Update copyright year

47217bb

alexeysemenyukoracle force-pushed the jlint-tests branch from b09c4b4 to 47217bb Compare February 6, 2025 18:04

alexeysemenyukoracle marked this pull request as ready for review February 6, 2025 18:07

openjdk bot added the rfr Pull request is ready for review label Feb 6, 2025

More cleanup

f60c3c5

sashamatveev reviewed Feb 6, 2025

View reviewed changes

Typo fixed

1354a8b

sashamatveev approved these changes Feb 12, 2025

View reviewed changes

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

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

openjdk bot closed this Feb 13, 2025

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

		* @library /test/jdk/tools/jpackage/helpers
		* @build jdk.jpackage.test.*

8349564: Clean warnings found in jpackage tests when building them with -Xlint:all #23455

8349564: Clean warnings found in jpackage tests when building them with -Xlint:all #23455

Uh oh!

Conversation

alexeysemenyukoracle commented Feb 5, 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 Feb 5, 2025

Uh oh!

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

Uh oh!

Uh oh!

openjdk bot commented Feb 5, 2025

Uh oh!

openjdk bot commented Feb 6, 2025

Uh oh!

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

Uh oh!

Webrevs

Uh oh!

sashamatveev Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

alexeysemenyukoracle Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

sashamatveev Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

alexeysemenyukoracle Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

sashamatveev Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

alexeysemenyukoracle Feb 6, 2025

Choose a reason for hiding this comment

Uh oh!

alexeysemenyukoracle commented Feb 12, 2025

Uh oh!

sashamatveev left a comment

Choose a reason for hiding this comment

Uh oh!

alexeysemenyukoracle commented Feb 13, 2025

Uh oh!

openjdk bot commented Feb 13, 2025

Uh oh!

openjdk bot commented Feb 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

alexeysemenyukoracle commented Feb 5, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Feb 5, 2025 •

edited

Loading

mlbridge bot commented Feb 6, 2025 •

edited

Loading