diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index 2b1dea9a04519f..ce267ea444b3ad 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -366,6 +366,12 @@ private static BuildEventStreamProtos.TestResult.ExecutionInfo extractExecutionI BuildEventStreamProtos.TestResult.ExecutionInfo.Builder executionInfo = BuildEventStreamProtos.TestResult.ExecutionInfo.newBuilder(); + // The return of `SpawnResult#exitCode()` is noted to only be meaningful if the subprocess + // actually executed. In this position, `spawnResult.exitCode()` is always meaningful, + // because the code only runs if `spawnResult.setupSuccess()` is previously verified to + // be `true`. + executionInfo.setExitCode(spawnResult.exitCode()); + if (spawnResult.isCacheHit()) { result.setRemotelyCached(true); executionInfo.setCachedRemotely(true); diff --git a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java index b106b0daa43c6d..5d676db4a45252 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java @@ -331,6 +331,7 @@ public void testRunTestOnce() throws Exception { assertThat(result.isCached()).isFalse(); assertThat(result.getTestAction()).isSameInstanceAs(testRunnerAction); assertThat(result.getData().getTestPassed()).isTrue(); + assertThat(result.getData().getExitCode()).isEqualTo(0); assertThat(result.getData().getRemotelyCached()).isFalse(); assertThat(result.getData().getIsRemoteStrategy()).isFalse(); assertThat(result.getData().getRunDurationMillis()).isEqualTo(10); @@ -411,6 +412,7 @@ public void testRunFlakyTest() throws Exception { assertThat(result.getTestAction()).isSameInstanceAs(testRunnerAction); assertThat(result.getData().getStatus()).isEqualTo(BlazeTestStatus.FLAKY); assertThat(result.getData().getTestPassed()).isTrue(); + assertThat(result.getData().getExitCode()).isEqualTo(0); assertThat(result.getData().getRemotelyCached()).isFalse(); assertThat(result.getData().getIsRemoteStrategy()).isFalse(); assertThat(result.getData().getRunDurationMillis()).isEqualTo(15L); @@ -425,9 +427,11 @@ public void testRunFlakyTest() throws Exception { assertThat(failedAttempt.getExecutionInfo().getStrategy()).isEqualTo("test"); assertThat(failedAttempt.getExecutionInfo().getHostname()).isEqualTo(""); assertThat(failedAttempt.getStatus()).isEqualTo(TestStatus.FAILED); + assertThat(failedAttempt.getExecutionInfo().getExitCode()).isEqualTo(1); assertThat(failedAttempt.getExecutionInfo().getCachedRemotely()).isFalse(); TestAttempt okAttempt = attempts.get(1); assertThat(okAttempt.getStatus()).isEqualTo(TestStatus.PASSED); + assertThat(okAttempt.getExecutionInfo().getExitCode()).isEqualTo(0); assertThat(okAttempt.getExecutionInfo().getStrategy()).isEqualTo("test"); assertThat(okAttempt.getExecutionInfo().getHostname()).isEqualTo(""); } @@ -483,6 +487,7 @@ public void testRunTestRemotely() throws Exception { assertThat(result.isCached()).isFalse(); assertThat(result.getTestAction()).isSameInstanceAs(testRunnerAction); assertThat(result.getData().getTestPassed()).isTrue(); + assertThat(result.getData().getExitCode()).isEqualTo(0); assertThat(result.getData().getRemotelyCached()).isFalse(); assertThat(result.getData().getIsRemoteStrategy()).isTrue(); assertThat(result.getData().getRunDurationMillis()).isEqualTo(10); @@ -495,6 +500,7 @@ public void testRunTestRemotely() throws Exception { .map(TestAttempt.class::cast) .collect(MoreCollectors.onlyElement()); assertThat(attempt.getStatus()).isEqualTo(TestStatus.PASSED); + assertThat(attempt.getExecutionInfo().getExitCode()).isEqualTo(0); assertThat(attempt.getExecutionInfo().getStrategy()).isEqualTo("remote"); assertThat(attempt.getExecutionInfo().getHostname()).isEqualTo("a-remote-host"); } @@ -551,6 +557,7 @@ public void testRunRemotelyCachedTest() throws Exception { assertThat(result.isCached()).isFalse(); assertThat(result.getTestAction()).isSameInstanceAs(testRunnerAction); assertThat(result.getData().getTestPassed()).isTrue(); + assertThat(result.getData().getExitCode()).isEqualTo(0); assertThat(result.getData().getRemotelyCached()).isTrue(); assertThat(result.getData().getIsRemoteStrategy()).isFalse(); assertThat(result.getData().getRunDurationMillis()).isEqualTo(10); @@ -911,6 +918,7 @@ public void testExperimentalCancelConcurrentTests() throws Exception { assertThat(standaloneTestStrategy.postedResult).isNotNull(); assertThat(standaloneTestStrategy.postedResult.getData().getStatus()) .isEqualTo(BlazeTestStatus.PASSED); + assertThat(standaloneTestStrategy.postedResult.getData().getExitCode()).isEqualTo(0); assertThat(storedEvents.getEvents()) .contains(Event.of(EventKind.PASS, null, "//standalone:empty_test (run 1 of 2)")); // Reset postedResult. @@ -1010,6 +1018,7 @@ public void testExperimentalCancelConcurrentTestsDoesNotTriggerOnFailedRun() thr assertThat(standaloneTestStrategy.postedResult).isNotNull(); assertThat(standaloneTestStrategy.postedResult.getData().getStatus()) .isEqualTo(BlazeTestStatus.FAILED); + assertThat(standaloneTestStrategy.postedResult.getData().getExitCode()).isEqualTo(1); assertContainsPrefixedEvent( storedEvents.getEvents(), Event.of(EventKind.FAIL, null, "//standalone:empty_test (run 1 of 2)")); @@ -1030,6 +1039,7 @@ public void testExperimentalCancelConcurrentTestsDoesNotTriggerOnFailedRun() thr assertThat(standaloneTestStrategy.postedResult).isNotNull(); assertThat(standaloneTestStrategy.postedResult.getData().getStatus()) .isEqualTo(BlazeTestStatus.PASSED); + assertThat(standaloneTestStrategy.postedResult.getData().getExitCode()).isEqualTo(0); assertThat(storedEvents.getEvents()) .contains(Event.of(EventKind.PASS, null, "//standalone:empty_test (run 2 of 2)")); } @@ -1116,6 +1126,7 @@ public void testExperimentalCancelConcurrentTestsAllFailed() throws Exception { assertThat(standaloneTestStrategy.postedResult).isNotNull(); assertThat(standaloneTestStrategy.postedResult.getData().getStatus()) .isEqualTo(BlazeTestStatus.FAILED); + assertThat(standaloneTestStrategy.postedResult.getData().getExitCode()).isEqualTo(1); assertContainsPrefixedEvent( storedEvents.getEvents(), Event.of(EventKind.FAIL, null, "//standalone:empty_test (run 1 of 2)")); @@ -1179,5 +1190,6 @@ public void missingTestLogSpawnTestResultIsIncomplete() throws Exception { assertThat(failedResult).isInstanceOf(StandaloneProcessedAttemptResult.class); TestResultData data = ((StandaloneProcessedAttemptResult) failedResult).testResultData(); assertThat(data.getStatus()).isEqualTo(BlazeTestStatus.INCOMPLETE); + assertThat(data.getExitCode()).isEqualTo(0); } }