RFR: 8281634: jdeps: java.lang.InternalError: Missing message: err.invalid.filters

[Jaikiran Pai]

On Mon, 14 Feb 2022 05:27:39 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

> Can I please get a review for this change which proposes to fix the issue noted in https://bugs.openjdk.java.net/browse/JDK-8281634?
> 
> The commit introduces the missing `err.invalid.filters` key in the jdeps resource bundle. I have run a simple check to make sure this resource bundle doesn't miss any other `err.` keys. From a simple search, following are the  unique `err.` keys used in the code of jdeps (under `src/jdk.jdeps/share/classes/com/sun/tools/jdeps` directory):
> 
> err.exception.message
> err.invalid.options
> err.multirelease.version.associated
> err.missing.arg
> err.multirelease.jar.malformed
> err.option.already.specified
> err.missing.dependences
> err.module.not.found
> err.invalid.path
> err.genmoduleinfo.not.jarfile
> err.invalid.arg.for.option
> err.multirelease.option.notfound
> err.filter.not.specified
> err.unknown.option
> err.command.set
> err.invalid.filters
> err.genmoduleinfo.unnamed.package
> err.option.after.class
> 
> Apart from the `err.invalid.filters` key which this PR is fixing, none of the other keys are missing from the resource bundle. I haven't updated the resource bundles for Japanese and Chinese languages because I don't know their equivalent values and looking at the commit history of those files, it appears that those changes are done as separate tasks (should a JBS issue be raised for this?)
> 
> The existing `test/langtools/tools/jdeps/Options.java` jtreg test has been updated to reproduce the issue and verify this fix.
> 
> An important detail about the update to the test is that while working on the update to this test, I realized that the current implementation in the test isn't fully functional. As a result, this test is currently, incorrectly considered as passing. Specifically, the test was missing a `assertTrue` call against the ouput/error content generated by the run of the `jdeps` tool.  This PR adds that assertion.
> Once that assertion is added, it started showing up 3 genuine failures. These failures are within that test code:
> - The test uses `-jdkinternal` instead of `-jdkinternals`. This appears to be a typo when this section in the test was added. The commit history of the source of jdeps tool shows that this option was always `-jdkinternals`. This PR fixes that part in the test.
> - The test expects an error message "-R cannot be used with --inverse option" when `-R` and `--inverse` are used together. However, at some point the source of jdeps tool was changed (as part of https://bugs.openjdk.java.net/browse/JDK-8213909) to return a different error message. That changes appears to have missed changing this test case error message and since this test has been falsely passing, it never got caught. This PR now fixes that issue by expecting the correct error message.
> - The test was expecting an error message "--list-deps and --list-reduced-deps options are specified" when "--list-deps" was used along with "--summary". This appears to be a copy/paste error in the test case and wasn't caught because the test was falsely passing. This too has been fixed in this PR.
> 
> With the fixes in this test, the test now reproduces the original issue and verifies the fix. I realize that this PR has changes in the test that aren't directly related to the issue raised in https://bugs.openjdk.java.net/browse/JDK-8281634, but those changes are necessary to get the test functional. However if a separate JBS issue needs to be opened to track those changes, please do let me know and I'll address these test changes in a separate PR.

Thank you for your review, Daniel.

>  Maybe wait one day or two before integrating to give Mandy a chance to chime in.

Certainly.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7455