IllegalStateException when command executed by jpackage "Executor" timeouts#31032
IllegalStateException when command executed by jpackage "Executor" timeouts#31032sashamatveev wants to merge 1 commit intoopenjdk:masterfrom
Conversation
|
@alexeysemenyukoracle Please review. |
|
👋 Welcome back almatvee! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@sashamatveev The following label will be automatically applied to this pull request:
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. |
| // Verify UnavailableExitCodeException | ||
| var expectExitCodeEx = assertThrowsExactly(UnavailableExitCodeException.class, () -> { | ||
| result.expectExitCode(0); | ||
| }); | ||
| assertEquals(String.format("Exit code unavailable from executing the command %s", | ||
| result.execAttrs().printableCommandLine()), expectExitCodeEx.getMessage()); | ||
|
|
There was a problem hiding this comment.
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.
| throw uecex; // Pass to exception mapper | ||
| } | ||
| // Pass exception to caller | ||
| throw ExceptionBox.toUnchecked(ex); |
There was a problem hiding this comment.
// 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();
UnavailableExitCodeExceptionwhich will be thrown when command timeouts to signal that exit code is not available.UnavailableExitCodeException.CodesignExceptionandretryUntilExitCodeIsadjusted for new exception.Progress
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/31032/head:pull/31032$ git checkout pull/31032Update a local copy of the PR:
$ git checkout pull/31032$ git pull https://git.openjdk.org/jdk.git pull/31032/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 31032View PR using the GUI difftool:
$ git pr show -t 31032Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/31032.diff