Unconditionally include the "message", if any, in SimpleSpawnResult. This is the same logic as used in Google-internal code. This allows us to stop passing the verboseFailures boolean around everywhere, since it's then only needed in one place in AbstractSpawnStrategy.
I don't think this justifies a RELNOTES, but ok if you'd prefer one.
#11151
PiperOrigin-RevId: 333156319
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 2e9b0ff..108bf66 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
@@ -41,7 +41,7 @@
.setExitCode(SpawnResult.POSIX_TIMEOUT_EXIT_CODE)
.setRunnerName("test")
.build();
- assertThat(r.getDetailMessage("", "", false, false, false))
+ assertThat(r.getDetailMessage("", "", false, false))
.contains("(failed due to timeout after 5.00 seconds.)");
}
@@ -53,8 +53,7 @@
.setExitCode(SpawnResult.POSIX_TIMEOUT_EXIT_CODE)
.setRunnerName("test")
.build();
- assertThat(r.getDetailMessage("", "", false, false, false))
- .contains("(failed due to timeout.)");
+ assertThat(r.getDetailMessage("", "", false, false)).contains("(failed due to timeout.)");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java b/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java
index 54a2dd1..fa8a23b 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java
@@ -54,11 +54,6 @@
}
@Override
- public boolean getVerboseFailures() {
- throw new UnsupportedOperationException();
- }
-
- @Override
public <T extends ActionContext> T getContext(Class<T> type) {
throw new UnsupportedOperationException();
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
index 0dfd3e2..08815f3 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
@@ -138,7 +138,7 @@
@Override
public ActionExecutionException toActionExecutionException(
- String messagePrefix, boolean verboseFailures, Action action) {
+ String messagePrefix, Action action) {
String message = messagePrefix + getMessage();
// Append cause.getMessage() if it's different from getMessage(). It typically
// isn't but if it is we'd like to surface cause.getMessage() as part of the
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 43c2ed5..1e503ab 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
@@ -66,7 +66,7 @@
public class AbstractSpawnStrategyTest {
private static class TestedSpawnStrategy extends AbstractSpawnStrategy {
public TestedSpawnStrategy(Path execRoot, SpawnRunner spawnRunner) {
- super(execRoot, spawnRunner);
+ super(execRoot, spawnRunner, /*verboseFailures=*/ true);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java
index 052eb9a..3ade294 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistryTest.java
@@ -567,7 +567,7 @@
private int usedCalled = 0;
public NoopAbstractStrategy(String name) {
- super(null, null);
+ super(null, null, /*verboseFailures=*/ true);
this.name = name;
}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
index 375fb2da..46dc051 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
@@ -213,7 +213,8 @@
}
private FakeSpawnExecutionContext getSpawnContext(Spawn spawn) {
- AbstractSpawnStrategy fakeLocalStrategy = new AbstractSpawnStrategy(execRoot, localRunner) {};
+ AbstractSpawnStrategy fakeLocalStrategy =
+ new AbstractSpawnStrategy(execRoot, localRunner, /*verboseFailures=*/ true) {};
ClassToInstanceMap<ActionContext> actionContextRegistry =
ImmutableClassToInstanceMap.of(RemoteLocalFallbackRegistry.class, () -> fakeLocalStrategy);
return new FakeSpawnExecutionContext(
@@ -821,7 +822,7 @@
SpawnResult result = runner.exec(spawn, policy);
assertThat(result.exitCode()).isEqualTo(ExitCode.REMOTE_ERROR.getNumericExitCode());
- assertThat(result.getDetailMessage("", "", false, false, false)).contains("reasons");
+ assertThat(result.getDetailMessage("", "", false, false)).contains("reasons");
}
@Test
@@ -841,7 +842,7 @@
SpawnResult result = runner.exec(spawn, policy);
assertThat(result.exitCode()).isEqualTo(ExitCode.REMOTE_ERROR.getNumericExitCode());
- assertThat(result.getDetailMessage("", "", false, false, false)).contains("reasons");
+ assertThat(result.getDetailMessage("", "", false, false)).contains("reasons");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
index 32154dc..eefdbc8 100644
--- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -151,7 +151,8 @@
(env, binTools1, fallbackTmpDir) -> ImmutableMap.copyOf(env),
binTools,
/*processWrapper=*/ null,
- Mockito.mock(RunfilesTreeUpdater.class)));
+ Mockito.mock(RunfilesTreeUpdater.class)),
+ /*verboseFailures=*/ false);
this.executor =
new TestExecutorBuilder(fileSystem, directories, binTools)
.addStrategy(strategy, "standalone")
@@ -313,8 +314,7 @@
@Test
public void testVerboseFailures() {
ExecException e = assertThrows(ExecException.class, () -> run(createSpawn(getFalseCommand())));
- ActionExecutionException actionExecutionException =
- e.toActionExecutionException("", /* verboseFailures= */ true, null);
+ ActionExecutionException actionExecutionException = e.toActionExecutionException("", null);
assertWithMessage("got: " + actionExecutionException.getMessage())
.that(actionExecutionException.getMessage().contains("failed: error executing command"))
.isTrue();
