On Thu, 21 Aug 2025 00:19:49 GMT, Alexey Semenyuk <asemen...@openjdk.org> wrote:
>> Alexander Matveev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8356218: [macos] Document --app-content [v3]
>
> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java line
> 748:
>
>> 746: ignoreExitCode = v;
>> 747: return this;
>> 748: }
>
> This is bad. It makes unclear semantics of the `JPackageCommand.execute(int
> expectedExitCode)` function.
>
> I suggest adding `JPackageCommand.executeIgnoreExitCode()` function instead
> in the following way:
>
> public Executor.Result executeIgnoreExitCode() {
> return execute(Optional.empty());
> }
>
> public Executor.Result execute(int expectedExitCode) {
> return execute(Optional.of(expectedExitCode));
> }
>
> private Executor.Result execute(Optional expectedExitCode) {
> // Here goes the implementation of the current execute(int
> expectedExitCode) function adjusted accordingly
> }
Fixed.
> test/jdk/tools/jpackage/share/AppContentTest.java line 122:
>
>> 120: @Parameter({TEST_DIR, "warning.non.standard.contents.sub.dir"})
>> 121: @Parameter({TEST_DUKE, "warning.app.content.is.not.dir"})
>> 122: public void testWarnings(String... args) throws Exception {
>
> `String... args` -> `String testPath, String warningId`
Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26848#discussion_r2289714888
PR Review Comment: https://git.openjdk.org/jdk/pull/26848#discussion_r2289715011
