HostGTest: Handle test failures properly
GTest runs return 1 if there are any test failures, but HostGTest fails
to parse any test output at all when this happens, making it appear as
if the whole test command had failed.
Bug: 123529934
Bug: 120164166
Test: m -j tradefed-all &&
tools/tradefederation/core/tests/run_tradefed_aosp_presubmit.sh
Test: 'cd external/avb && atest libavb_host_unittest' (which currently
has some failing test cases) now reports the test failures correctly,
instead of saying the whole test run errored out.
Change-Id: Iabf93e1debe075819f3edcc7720104786bf7d26a
Merged-In: Iabf93e1debe075819f3edcc7720104786bf7d26a
diff --git a/src/com/android/tradefed/util/CommandResult.java b/src/com/android/tradefed/util/CommandResult.java
index e3393d3..867ecb7 100644
--- a/src/com/android/tradefed/util/CommandResult.java
+++ b/src/com/android/tradefed/util/CommandResult.java
@@ -24,6 +24,7 @@
private CommandStatus mCmdStatus = CommandStatus.TIMED_OUT;
private String mStdout = null;
private String mStderr = null;
+ private Integer mExitCode = null;
/**
* Create a {@link CommandResult} with the default {@link CommandStatus#TIMED_OUT} status.
@@ -78,4 +79,17 @@
public void setStderr(String stderr) {
mStderr = stderr;
}
-}
\ No newline at end of file
+
+ /**
+ * Get the exit/return code produced by command.
+ *
+ * @return the exit code or <code>null</code> if it is unset
+ */
+ public Integer getExitCode() {
+ return mExitCode;
+ }
+
+ public void setExitCode(int exitCode) {
+ mExitCode = exitCode;
+ }
+}
diff --git a/src/com/android/tradefed/util/RunUtil.java b/src/com/android/tradefed/util/RunUtil.java
index 8737a0c..03bf49e 100644
--- a/src/com/android/tradefed/util/RunUtil.java
+++ b/src/com/android/tradefed/util/RunUtil.java
@@ -137,9 +137,9 @@
@Override
public CommandResult runTimedCmd(final long timeout, OutputStream stdout,
OutputStream stderr, final String... command) {
- final CommandResult result = new CommandResult();
- IRunUtil.IRunnableResult osRunnable = createRunnableResult(result, stdout, stderr, command);
+ RunnableResult osRunnable = createRunnableResult(stdout, stderr, command);
CommandStatus status = runTimed(timeout, osRunnable, true);
+ CommandResult result = osRunnable.getResult();
result.setStatus(status);
return result;
}
@@ -149,12 +149,9 @@
* command.
*/
@VisibleForTesting
- IRunUtil.IRunnableResult createRunnableResult(
- CommandResult result,
- OutputStream stdout,
- OutputStream stderr,
- String... command) {
- return new RunnableResult(result, null, createProcessBuilder(command), stdout, stderr);
+ RunnableResult createRunnableResult(
+ OutputStream stdout, OutputStream stderr, String... command) {
+ return new RunnableResult(null, createProcessBuilder(command), stdout, stderr);
}
/** {@inheritDoc} */
@@ -220,10 +217,9 @@
@Override
public CommandResult runTimedCmdWithInput(final long timeout, String input,
final List<String> command) {
- final CommandResult result = new CommandResult();
- IRunUtil.IRunnableResult osRunnable = new RunnableResult(result, input,
- createProcessBuilder(command));
+ RunnableResult osRunnable = new RunnableResult(input, createProcessBuilder(command));
CommandStatus status = runTimed(timeout, osRunnable, true);
+ CommandResult result = osRunnable.getResult();
result.setStatus(status);
return result;
}
@@ -233,10 +229,9 @@
*/
@Override
public CommandResult runTimedCmdSilently(final long timeout, final String... command) {
- final CommandResult result = new CommandResult();
- IRunUtil.IRunnableResult osRunnable = new RunnableResult(result, null,
- createProcessBuilder(command));
+ RunnableResult osRunnable = new RunnableResult(null, createProcessBuilder(command));
CommandStatus status = runTimed(timeout, osRunnable, false);
+ CommandResult result = osRunnable.getResult();
result.setStatus(status);
return result;
}
@@ -537,9 +532,8 @@
private final Object mLock = new Object();
private boolean mCancelled = false;
- RunnableResult(final CommandResult result, final String input,
- final ProcessBuilder processBuilder) {
- this(result, input, processBuilder, null, null);
+ RunnableResult(final String input, final ProcessBuilder processBuilder) {
+ this(input, processBuilder, null, null);
}
/**
@@ -548,14 +542,13 @@
* streams are null, default behavior of using a buffer will be used.
*/
RunnableResult(
- final CommandResult result,
final String input,
final ProcessBuilder processBuilder,
OutputStream stdoutStream,
OutputStream stderrStream) {
mProcessBuilder = processBuilder;
mInput = input;
- mCommandResult = result;
+ mCommandResult = new CommandResult();
// Ensure the outputs are never null
mCommandResult.setStdout("");
mCommandResult.setStderr("");
@@ -576,6 +569,10 @@
}
}
+ public CommandResult getResult() {
+ return mCommandResult;
+ }
+
/** Start a {@link Process} based on the {@link ProcessBuilder}. */
@VisibleForTesting
Process startProcess() throws IOException {
@@ -624,7 +621,7 @@
}
}
// Wait for process to complete.
- int rc = Integer.MIN_VALUE;
+ Integer rc = null;
try {
try {
rc = mProcess.waitFor();
@@ -638,25 +635,27 @@
CLog.d("stderr read thread %s still alive.", stderrThread.toString());
}
} finally {
+ mCommandResult.setExitCode(rc);
+
// Write out the streams to the result.
if (stdOut instanceof ByteArrayOutputStream) {
mCommandResult.setStdout(((ByteArrayOutputStream)stdOut).toString("UTF-8"));
} else {
- mCommandResult.setStdout("redirected to " +
- stdOut.getClass().getSimpleName());
+ mCommandResult.setStdout(
+ "redirected to " + stdOut.getClass().getSimpleName());
}
if (stdErr instanceof ByteArrayOutputStream) {
mCommandResult.setStderr(((ByteArrayOutputStream)stdErr).toString("UTF-8"));
} else {
- mCommandResult.setStderr("redirected to " +
- stdErr.getClass().getSimpleName());
+ mCommandResult.setStderr(
+ "redirected to " + stdErr.getClass().getSimpleName());
}
}
} finally {
mCountDown.countDown();
}
- if (rc == 0) {
+ if (rc != null && rc == 0) {
return true;
} else {
CLog.d("%s command failed. return code %d", mProcessBuilder.command(), rc);
diff --git a/tests/src/com/android/tradefed/util/RunUtilTest.java b/tests/src/com/android/tradefed/util/RunUtilTest.java
index 2fe1336..78b6e14 100644
--- a/tests/src/com/android/tradefed/util/RunUtilTest.java
+++ b/tests/src/com/android/tradefed/util/RunUtilTest.java
@@ -71,12 +71,9 @@
}
@Override
- IRunnableResult createRunnableResult(
- CommandResult result,
- OutputStream stdout,
- OutputStream stderr,
- String... command) {
- IRunnableResult real = super.createRunnableResult(result, stdout, stderr, command);
+ RunnableResult createRunnableResult(
+ OutputStream stdout, OutputStream stderr, String... command) {
+ RunnableResult real = super.createRunnableResult(stdout, stderr, command);
mMockRunnableResult = (RunnableResult) Mockito.spy(real);
try {
if (mShouldThrow) {
@@ -484,6 +481,15 @@
assertEquals(expected + "\n", result.getStdout());
}
+ public void testGotExitCodeFromCommand() {
+ RunUtil testRunUtil = new RunUtil();
+ CommandResult result =
+ testRunUtil.runTimedCmd(VERY_LONG_TIMEOUT_MS, "/bin/bash", "-c", "exit 2");
+ assertEquals("", result.getStdout());
+ assertEquals("", result.getStderr());
+ assertEquals(2, (int) result.getExitCode());
+ }
+
/**
* Implementation of {@link Process} to simulate a success of 'echo Test' without actually
* calling the underlying system.
