On Tue, 5 May 2026 01:13:36 GMT, Alexander Matveev <[email protected]> wrote:

> - Fixed by introducing `UnavailableExitCodeException` which will be thrown 
> when command timeouts to signal that exit code is not available.
> - Added test coverage for `UnavailableExitCodeException`.
> - `CodesignException` and `retryUntilExitCodeIs` adjusted for new exception.
> 
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

Changes requested by asemenyuk (Reviewer).

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java line 382:

> 380:                 }
> 381:                 // Pass exception to caller
> 382:                 throw ExceptionBox.toUnchecked(ex);

`// Pass exception to caller` comment is misleading because control flow can't 
reach `throw ExceptionBox.toUnchecked(ex)` expression because the Executor 
class doesn't execute commands with timeouts.

I'd replace it with:

// Unreachable, because the result must always have the exit code, as the 
executor never runs commands with a timeout.
throw ExceptionBox.reachedUnreachable();

test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/CommandOutputControlTest.java
 line 360:

> 358:         assertEquals(String.format("Exit code unavailable from executing 
> the command %s",
> 359:                 result.execAttrs().printableCommandLine()), 
> expectExitCodeEx.getMessage());
> 360: 

It is good to have this supplementary check.

However, we need a dedicated unit test that covers the behavior of 
`Result.expectExitCode()` when the Result instance doesn't have an exit code. 
You can model one from `test_Result_expectExitCode()` and 
`test_Result_expectExitCode_negative()` unit tests.

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

PR Review: https://git.openjdk.org/jdk/pull/31032#pullrequestreview-4224904397
PR Review Comment: https://git.openjdk.org/jdk/pull/31032#discussion_r3185703001
PR Review Comment: https://git.openjdk.org/jdk/pull/31032#discussion_r3185607924

Reply via email to