RFR: 8321139: jlink's man page does not document the --compress option correctly [v4]

Ana Maria Mihalceanu duke at openjdk.org
Fri Nov 21 18:45:58 UTC 2025


On Fri, 21 Nov 2025 10:35:50 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Ana Maria Mihalceanu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add some negative tests for suggested values.
>
> test/jdk/tools/jlink/TaskHelperTest.java line 279:
> 
>> 277:                 var remaining = optionsHelper.handleOptions(this, args.tokens);
>> 278:                 assertTrue(remaining.isEmpty());
>> 279:                 assertEquals(args.expectedCompressValue, compressArgValue);
> 
> I think these 2 asserts are an oversight. Since we expect `optionsHelper.handleOptions(...)` to always throw a `BadArgs` for these invalid option values, we should replace these 2 asserts with a:
> 
> fail("processing of " + Arrays.toString(testCase.tokens) + " was expected to fail, but didn't");
> 
> This will then rightly fail the test if the exception isn't thrown.

To my understanding, for compression plugin the validation of the actual value is performed in `DefaultCompressPlugin` (https://github.com/openjdk/jdk/blob/d57fc1b6dc313eb004892b180960ebcee1cb04c7/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/DefaultCompressPlugin.java#L118 and https://github.com/openjdk/jdk/blob/d57fc1b6dc313eb004892b180960ebcee1cb04c7/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/DefaultCompressPlugin.java#L115). 

At the moment, [`optionsHelper.handleOptions`](https://github.com/openjdk/jdk/blob/3a45e615973727446c9081b5affbbe7ffe7c3bea/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/TaskHelper.java#L527) seems to set up `TaskHelper`'s internal state and it shouldn't throw exceptions for syntactically correct options. In my opinion, this fix is not that much related to the value allowed, but to the syntax allowed for the option. I noticed that there is another test class [`JLinkTest`](https://github.com/openjdk/jdk/blob/master/test/jdk/tools/jlink/JLinkTest.java) that tests for actual values. When I tested locally my build of this modified `jlink`, for invalid values I did get that `IllegalArgumentException`. 

However, I am very new to the testing framework and am welcoming more thoughts/ideas 🙏 .

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28359#discussion_r2550661428


More information about the core-libs-dev mailing list