From 8dae670425549f67ad57a822ace11b6049cada48 Mon Sep 17 00:00:00 2001 From: Peter Palaga Date: Sun, 15 Nov 2020 15:01:46 +0100 Subject: [PATCH] Exit code not propagated from the daemon to mvnd client #220 --- .../mvndaemon/mvnd/client/DefaultClient.java | 42 ++++++++++++++----- .../mvnd/client/ExecutionResult.java | 2 + .../org/mvndaemon/mvnd/common/Message.java | 37 +++++++++++++--- .../mvnd/common/logging/TerminalOutput.java | 2 +- .../org/mvndaemon/mvnd/daemon/Server.java | 10 ++--- .../mvnd/it/NewManagedModuleNativeIT.java | 11 +---- .../mvnd/it/UpgradesInBomNativeIT.java | 3 +- .../mvnd/junit/NativeTestClient.java | 4 ++ 8 files changed, 80 insertions(+), 31 deletions(-) diff --git a/client/src/main/java/org/mvndaemon/mvnd/client/DefaultClient.java b/client/src/main/java/org/mvndaemon/mvnd/client/DefaultClient.java index 7981f16ff..7caa30d26 100644 --- a/client/src/main/java/org/mvndaemon/mvnd/client/DefaultClient.java +++ b/client/src/main/java/org/mvndaemon/mvnd/client/DefaultClient.java @@ -41,6 +41,7 @@ import org.mvndaemon.mvnd.common.Environment; import org.mvndaemon.mvnd.common.Message; import org.mvndaemon.mvnd.common.Message.BuildException; +import org.mvndaemon.mvnd.common.Message.BuildFinished; import org.mvndaemon.mvnd.common.OsUtils; import org.mvndaemon.mvnd.common.TimeUtils; import org.mvndaemon.mvnd.common.logging.ClientOutput; @@ -79,16 +80,20 @@ public static void main(String[] argv) throws Exception { } DaemonParameters parameters = new DaemonParameters(); + int exitCode = 0; try (TerminalOutput output = new TerminalOutput(batchMode || parameters.noBuffering(), parameters.rollingWindowSize(), logFile)) { try { - new DefaultClient(parameters).execute(output, args); + final ExecutionResult result = new DefaultClient(parameters).execute(output, args); + exitCode = result.getExitCode(); } catch (DaemonException.InterruptedException e) { final AttributedStyle s = new AttributedStyle().bold().foreground(AttributedStyle.RED); String str = new AttributedString(System.lineSeparator() + "Canceled by user", s).toAnsi(); output.accept(Message.display(str)); + exitCode = 130; } } + System.exit(exitCode); } public DefaultClient(DaemonParameters parameters) { @@ -176,7 +181,7 @@ public ExecutionResult execute(ClientOutput output, List argv) { d.getJavaHome()))); } } - return new DefaultResult(argv, null); + return DefaultResult.success(argv); } boolean stop = args.remove("--stop"); if (stop) { @@ -193,13 +198,13 @@ public ExecutionResult execute(ClientOutput output, List argv) { } } } - return new DefaultResult(argv, null); + return DefaultResult.success(argv); } boolean purge = args.remove("--purge"); if (purge) { String result = purgeLogs(); output.accept(Message.display(result != null ? result : "Nothing to purge")); - return new DefaultResult(argv, null); + return DefaultResult.success(argv); } if (args.stream().noneMatch(arg -> arg.startsWith("-T") || arg.equals("--threads"))) { @@ -250,13 +255,14 @@ public ExecutionResult execute(ClientOutput output, List argv) { switch (m.getType()) { case Message.CANCEL_BUILD: return new DefaultResult(argv, - new InterruptedException("The build was canceled")); + new InterruptedException("The build was canceled"), 130); case Message.BUILD_EXCEPTION: final BuildException e = (BuildException) m; return new DefaultResult(argv, - new Exception(e.getClassName() + ": " + e.getMessage() + "\n" + e.getStackTrace())); - case Message.BUILD_STOPPED: - return new DefaultResult(argv, null); + new Exception(e.getClassName() + ": " + e.getMessage() + "\n" + e.getStackTrace()), + 1); + case Message.BUILD_FINISHED: + return new DefaultResult(argv, null, ((BuildFinished) m).getExitCode()); } } } @@ -344,11 +350,17 @@ private static class DefaultResult implements ExecutionResult { private final Exception exception; private final List args; + private final int exitCode; - private DefaultResult(List args, Exception exception) { + public static DefaultResult success(List args) { + return new DefaultResult(args, null, 0); + } + + private DefaultResult(List args, Exception exception, int exitCode) { super(); this.args = args; this.exception = exception; + this.exitCode = exitCode; } @Override @@ -356,17 +368,27 @@ public ExecutionResult assertSuccess() { if (exception != null) { throw new AssertionError(appendCommand(new StringBuilder("Build failed: ")).toString(), exception); } + if (exitCode != 0) { + throw new AssertionError( + appendCommand(new StringBuilder("Build exited with non-zero exit code " + exitCode + ": ")).toString(), + exception); + } return this; } @Override public ExecutionResult assertFailure() { - if (exception == null) { + if (exception == null && exitCode == 0) { throw new AssertionError(appendCommand(new StringBuilder("Build did not fail: "))); } return this; } + @Override + public int getExitCode() { + return exitCode; + } + @Override public boolean isSuccess() { return exception == null; diff --git a/client/src/main/java/org/mvndaemon/mvnd/client/ExecutionResult.java b/client/src/main/java/org/mvndaemon/mvnd/client/ExecutionResult.java index 60b657636..9fdf18980 100644 --- a/client/src/main/java/org/mvndaemon/mvnd/client/ExecutionResult.java +++ b/client/src/main/java/org/mvndaemon/mvnd/client/ExecutionResult.java @@ -26,4 +26,6 @@ public interface ExecutionResult { ExecutionResult assertSuccess(); + int getExitCode(); + } diff --git a/common/src/main/java/org/mvndaemon/mvnd/common/Message.java b/common/src/main/java/org/mvndaemon/mvnd/common/Message.java index 9784b5da8..f2eacc6f8 100644 --- a/common/src/main/java/org/mvndaemon/mvnd/common/Message.java +++ b/common/src/main/java/org/mvndaemon/mvnd/common/Message.java @@ -31,7 +31,7 @@ public abstract class Message { public static final int BUILD_REQUEST = 0; public static final int BUILD_STARTED = 1; - public static final int BUILD_STOPPED = 2; + public static final int BUILD_FINISHED = 2; public static final int PROJECT_STARTED = 3; public static final int PROJECT_STOPPED = 4; public static final int MOJO_STARTED = 5; @@ -49,7 +49,6 @@ public abstract class Message { public static final BareMessage KEEP_ALIVE_SINGLETON = new BareMessage(KEEP_ALIVE); public static final BareMessage STOP_SINGLETON = new BareMessage(STOP); - public static final BareMessage BUILD_STOPPED_SINGLETON = new BareMessage(BUILD_STOPPED); public static final BareMessage CANCEL_BUILD_SINGLETON = new BareMessage(CANCEL_BUILD); final int type; @@ -68,8 +67,8 @@ public static Message read(DataInputStream input) throws IOException { return BuildRequest.read(input); case BUILD_STARTED: return BuildStarted.read(input); - case BUILD_STOPPED: - return BareMessage.BUILD_STOPPED_SINGLETON; + case BUILD_FINISHED: + return BuildFinished.read(input); case PROJECT_STARTED: case PROJECT_STOPPED: case MOJO_STARTED: @@ -300,6 +299,34 @@ public void write(DataOutputStream output) throws IOException { } } + public static class BuildFinished extends Message { + final int exitCode; + + public static Message read(DataInputStream input) throws IOException { + return new BuildFinished(input.readInt()); + } + + public BuildFinished(int exitCode) { + super(BUILD_FINISHED); + this.exitCode = exitCode; + } + + @Override + public String toString() { + return "BuildRequest{exitCode=" + exitCode + '}'; + } + + @Override + public void write(DataOutputStream output) throws IOException { + super.write(output); + output.writeInt(exitCode); + } + + public int getExitCode() { + return exitCode; + } + } + public static class BuildException extends Message { final String message; final String className; @@ -474,7 +501,7 @@ public String toString() { switch (type) { case KEEP_ALIVE: return "KeepAlive"; - case BUILD_STOPPED: + case BUILD_FINISHED: return "BuildStopped"; case STOP: return "Stop"; diff --git a/common/src/main/java/org/mvndaemon/mvnd/common/logging/TerminalOutput.java b/common/src/main/java/org/mvndaemon/mvnd/common/logging/TerminalOutput.java index 5f1880fc7..e4438b48d 100644 --- a/common/src/main/java/org/mvndaemon/mvnd/common/logging/TerminalOutput.java +++ b/common/src/main/java/org/mvndaemon/mvnd/common/logging/TerminalOutput.java @@ -229,7 +229,7 @@ private boolean doAccept(Message entry) { this.buildStatus = ((StringMessage) entry).getMessage(); break; } - case Message.BUILD_STOPPED: { + case Message.BUILD_FINISHED: { projects.values().stream().flatMap(p -> p.log.stream()).forEach(log); clearDisplay(); try { diff --git a/daemon/src/main/java/org/mvndaemon/mvnd/daemon/Server.java b/daemon/src/main/java/org/mvndaemon/mvnd/daemon/Server.java index c5732800e..f17e25503 100644 --- a/daemon/src/main/java/org/mvndaemon/mvnd/daemon/Server.java +++ b/daemon/src/main/java/org/mvndaemon/mvnd/daemon/Server.java @@ -529,13 +529,13 @@ public T request(Message request, Class responseType, Pre } } }); - cli.main( + int exitCode = cli.main( buildRequest.getArgs(), buildRequest.getWorkingDir(), buildRequest.getProjectDir(), buildRequest.getEnv()); LOGGER.info("Build finished, finishing message dispatch"); - loggingSpy.finish(); + loggingSpy.finish(exitCode); } catch (Throwable t) { LOGGER.error("Error while building project", t); loggingSpy.fail(t); @@ -574,7 +574,7 @@ int getClassOrder(Message m) { return 51; case Message.PROJECT_STOPPED: return 95; - case Message.BUILD_STOPPED: + case Message.BUILD_FINISHED: return 96; case Message.BUILD_EXCEPTION: return 97; @@ -636,8 +636,8 @@ public DaemonLoggingSpy(BlockingQueue queue) { this.queue = queue; } - public void finish() throws Exception { - queue.add(Message.BUILD_STOPPED_SINGLETON); + public void finish(int exitCode) throws Exception { + queue.add(new Message.BuildFinished(exitCode)); queue.add(Message.STOP_SINGLETON); } diff --git a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/NewManagedModuleNativeIT.java b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/NewManagedModuleNativeIT.java index ea7229c49..d736f19df 100644 --- a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/NewManagedModuleNativeIT.java +++ b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/NewManagedModuleNativeIT.java @@ -19,8 +19,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; -import java.util.List; -import java.util.stream.Collectors; import java.util.stream.Stream; import javax.inject.Inject; import org.assertj.core.api.Assertions; @@ -84,13 +82,8 @@ void upgrade() throws IOException, InterruptedException { /* Build again */ { final TestClientOutput output = new TestClientOutput(); - cl.execute(output, "clean", "install", "-e", "-B", "-ntp").assertSuccess(); - - final List messagesToString = output.messagesToString(); - Assertions.assertThat(messagesToString.stream().noneMatch(m -> m.contains("[ERROR]"))) - .withFailMessage("Should contain no errors:\n %s", - messagesToString.stream().collect(Collectors.joining("\n "))) - .isTrue(); + cl.execute(output, "clean", "install", "-e", "-B", "-ntp") + .assertFailure(); // Switch back to assertSuccess() once https://github.com/mvndaemon/mvnd/issues/218 is fixed } Assertions.assertThat(registry.getAll().size()).isEqualTo(1); diff --git a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/UpgradesInBomNativeIT.java b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/UpgradesInBomNativeIT.java index 3dc3c2e8e..bc32a89ed 100644 --- a/integration-tests/src/test/java/org/mvndaemon/mvnd/it/UpgradesInBomNativeIT.java +++ b/integration-tests/src/test/java/org/mvndaemon/mvnd/it/UpgradesInBomNativeIT.java @@ -76,7 +76,8 @@ void upgrade() throws IOException, InterruptedException { TestUtils.replace(useHelloPath, "new Hello().sayHello()", "new Hello().sayWisdom()"); { final TestClientOutput output = new TestClientOutput(); - cl.execute(output, "clean", "install", "-e").assertSuccess(); + cl.execute(output, "clean", "install", "-e") + .assertFailure(); // Switch back to assertSuccess() once https://github.com/mvndaemon/mvnd/issues/218 is fixed } Assertions.assertThat(registry.getAll().size()).isEqualTo(1); diff --git a/integration-tests/src/test/java/org/mvndaemon/mvnd/junit/NativeTestClient.java b/integration-tests/src/test/java/org/mvndaemon/mvnd/junit/NativeTestClient.java index de1226a7a..ae8222b71 100644 --- a/integration-tests/src/test/java/org/mvndaemon/mvnd/junit/NativeTestClient.java +++ b/integration-tests/src/test/java/org/mvndaemon/mvnd/junit/NativeTestClient.java @@ -121,6 +121,7 @@ StringBuilder appendCommand(StringBuilder sb) { } + @Override public Result assertFailure() { if (exitCode == 0) { throw new AssertionError(appendCommand( @@ -129,6 +130,7 @@ public Result assertFailure() { return this; } + @Override public Result assertSuccess() { if (exitCode != 0) { final StringBuilder sb = appendCommand(new StringBuilder("mvnd returned ").append(exitCode)); @@ -145,10 +147,12 @@ public Result assertSuccess() { return this; } + @Override public int getExitCode() { return exitCode; } + @Override public boolean isSuccess() { return exitCode == 0; }