Stop manually overriding failure detail's coarse exit code in SpawnExecException#toDetailedExitCode. Instead, trust it. This requires changing some failure codes' associated coarse exit codes so that they are properly categorized: a code like Spawn#EXEC_FAILED that is associated with a "system" error status needs to have an exit code of 34 to reflect that.
#11151
PiperOrigin-RevId: 335648755
diff --git a/src/test/java/com/google/devtools/build/lib/actions/SpawnResultTest.java b/src/test/java/com/google/devtools/build/lib/actions/SpawnResultTest.java
index 108bf66..fee0304 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/SpawnResultTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/SpawnResultTest.java
@@ -18,6 +18,9 @@
import com.google.devtools.build.lib.actions.SpawnResult.MetadataLog;
import com.google.devtools.build.lib.actions.SpawnResult.Status;
+import com.google.devtools.build.lib.server.FailureDetails;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.util.FileSystems;
import com.google.protobuf.ByteString;
@@ -39,6 +42,10 @@
.setStatus(SpawnResult.Status.TIMEOUT)
.setWallTime(Duration.ofSeconds(5))
.setExitCode(SpawnResult.POSIX_TIMEOUT_EXIT_CODE)
+ .setFailureDetail(
+ FailureDetail.newBuilder()
+ .setSpawn(FailureDetails.Spawn.newBuilder().setCode(Code.TIMEOUT))
+ .build())
.setRunnerName("test")
.build();
assertThat(r.getDetailMessage("", "", false, false))
@@ -51,6 +58,10 @@
new SpawnResult.Builder()
.setStatus(SpawnResult.Status.TIMEOUT)
.setExitCode(SpawnResult.POSIX_TIMEOUT_EXIT_CODE)
+ .setFailureDetail(
+ FailureDetail.newBuilder()
+ .setSpawn(FailureDetails.Spawn.newBuilder().setCode(Code.TIMEOUT))
+ .build())
.setRunnerName("test")
.build();
assertThat(r.getDetailMessage("", "", false, false)).contains("(failed due to timeout.)");
@@ -75,7 +86,7 @@
}
@Test
- public void getSpawnResultLogs() throws Exception {
+ public void getSpawnResultLogs() {
SpawnResult.Builder builder =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setExitCode(0).setRunnerName("test");
diff --git a/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java
index 1e503ab..6901136 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java
@@ -42,6 +42,9 @@
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
import com.google.devtools.build.lib.exec.util.SpawnBuilder;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
+import com.google.devtools.build.lib.server.FailureDetails;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.testutil.Suite;
import com.google.devtools.build.lib.testutil.TestSpec;
@@ -64,6 +67,11 @@
@RunWith(JUnit4.class)
@TestSpec(size = Suite.SMALL_TESTS)
public class AbstractSpawnStrategyTest {
+ private static final FailureDetail NON_ZERO_EXIT_DETAILS =
+ FailureDetail.newBuilder()
+ .setSpawn(FailureDetails.Spawn.newBuilder().setCode(Code.NON_ZERO_EXIT))
+ .build();
+
private static class TestedSpawnStrategy extends AbstractSpawnStrategy {
public TestedSpawnStrategy(Path execRoot, SpawnRunner spawnRunner) {
super(execRoot, spawnRunner, /*verboseFailures=*/ true);
@@ -116,6 +124,7 @@
new SpawnResult.Builder()
.setStatus(Status.NON_ZERO_EXIT)
.setExitCode(1)
+ .setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("test")
.build();
when(spawnRunner.execAsync(any(Spawn.class), any(SpawnExecutionContext.class)))
@@ -188,6 +197,7 @@
new SpawnResult.Builder()
.setStatus(Status.NON_ZERO_EXIT)
.setExitCode(1)
+ .setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("test")
.build();
when(spawnRunner.execAsync(any(Spawn.class), any(SpawnExecutionContext.class)))
@@ -383,6 +393,7 @@
new SpawnResult.Builder()
.setStatus(Status.NON_ZERO_EXIT)
.setExitCode(23)
+ .setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("runner")
.build()));
when(actionExecutionContext.getMetadataProvider()).thenReturn(mock(MetadataProvider.class));
diff --git a/src/test/java/com/google/devtools/build/lib/exec/BUILD b/src/test/java/com/google/devtools/build/lib/exec/BUILD
index 1ef8109..4319a88 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD
@@ -65,6 +65,7 @@
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
"//src/main/java/com/google/devtools/common/options",
+ "//src/main/protobuf:failure_details_java_proto",
"//src/main/protobuf:spawn_java_proto",
"//src/main/protobuf:test_status_java_proto",
"//src/test/java/com/google/devtools/build/lib/actions/util",
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 b8cb751..be2f78c 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
@@ -59,6 +59,9 @@
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.exec.StandaloneTestStrategy.StandaloneFailedAttemptResult;
import com.google.devtools.build.lib.exec.util.TestExecutorBuilder;
+import com.google.devtools.build.lib.server.FailureDetails;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.util.io.FileOutErr;
@@ -85,6 +88,10 @@
/** Unit tests for {@link StandaloneTestStrategy}. */
@RunWith(JUnit4.class)
public final class StandaloneTestStrategyTest extends BuildViewTestCase {
+ private static final FailureDetail NON_ZERO_EXIT_DETAILS =
+ FailureDetail.newBuilder()
+ .setSpawn(FailureDetails.Spawn.newBuilder().setCode(Code.NON_ZERO_EXIT))
+ .build();
private static class TestedStandaloneTestStrategy extends StandaloneTestStrategy {
TestResult postedResult = null;
@@ -327,6 +334,7 @@
new SpawnResult.Builder()
.setStatus(Status.NON_ZERO_EXIT)
.setExitCode(1)
+ .setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setWallTime(Duration.ofMillis(10))
.setRunnerName("test")
.build();
@@ -522,6 +530,7 @@
new SpawnResult.Builder()
.setStatus(Status.NON_ZERO_EXIT)
.setExitCode(1)
+ .setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("test")
.build();
when(spawnStrategy.beginExecution(any(), any()))
@@ -614,6 +623,7 @@
new SpawnResult.Builder()
.setStatus(Status.NON_ZERO_EXIT)
.setExitCode(1)
+ .setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("test")
.build();
SpawnResult xmlGeneratorSpawnResult =
@@ -893,6 +903,7 @@
new SpawnResult.Builder()
.setStatus(Status.NON_ZERO_EXIT)
.setExitCode(1)
+ .setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("test")
.build();
when(spawnStrategy.beginExecution(any(), any()))
@@ -983,6 +994,7 @@
new SpawnResult.Builder()
.setStatus(Status.NON_ZERO_EXIT)
.setExitCode(1)
+ .setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("test")
.build();
when(spawnStrategy.beginExecution(any(), any()))
diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD
index 6e854c1..d8b77f6 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD
@@ -82,6 +82,7 @@
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
"//src/main/java/com/google/devtools/common/options",
+ "//src/main/protobuf:failure_details_java_proto",
"//src/main/protobuf:remote_execution_log_java_proto",
"//src/test/java/com/google/devtools/build/lib:test_runner",
"//src/test/java/com/google/devtools/build/lib/actions/util",
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
index 3ee5392..c385e45 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
@@ -36,7 +36,6 @@
import com.google.devtools.build.lib.actions.ActionContext;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputHelper;
-import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.MetadataProvider;
@@ -64,6 +63,9 @@
import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
+import com.google.devtools.build.lib.server.FailureDetails;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
@@ -93,12 +95,7 @@
@RunWith(JUnit4.class)
public class RemoteSpawnCacheTest {
private static final ArtifactExpander SIMPLE_ARTIFACT_EXPANDER =
- new ArtifactExpander() {
- @Override
- public void expand(Artifact artifact, Collection<? super Artifact> output) {
- output.add(artifact);
- }
- };
+ (artifact, output) -> output.add(artifact);
private FileSystem fs;
private DigestUtil digestUtil;
@@ -125,7 +122,7 @@
public void prefetchInputs() {}
@Override
- public void lockOutputFiles() throws InterruptedException {
+ public void lockOutputFiles() {
throw new UnsupportedOperationException();
}
@@ -453,6 +450,10 @@
new SpawnResult.Builder()
.setExitCode(1)
.setStatus(Status.NON_ZERO_EXIT)
+ .setFailureDetail(
+ FailureDetail.newBuilder()
+ .setSpawn(FailureDetails.Spawn.newBuilder().setCode(Code.NON_ZERO_EXIT))
+ .build())
.setRunnerName("test")
.build();
ImmutableList<Path> outputFiles = ImmutableList.of(fs.getPath("/random/file"));
