Add `--experimental_cancel_concurrent_tests=on_failed`
The existing `--experimental_cancel_concurrent_tests` flag can be used to let Bazel cancel concurrent runs of the same test as soon as one run passes with `--runs_per_test_detects_flakes`. This change turns the boolean-valued into an enum-valued flag that also accepts the value `on_failed` to instead cancel on the first failed run. This can be used to avoid reruns of flaky tests when the intention is to only detect flakes, not their probability of being flaky.
Also fix the quirk that `--experimental_cancel_concurrent_tests` has no effect with default Bazel settings since the `exclusive` test strategy is registered last and thus always wins. Perhaps surprisingly, this is the only effect of that incorrect registration order - all other `exclusive` logic is explicitly gated behind the value of `--test_strategy`.
RELNOTES[NEW]: The `--experimental_cancel_concurrent_tests` option now accepts the values `on_passed`, `on_failed` and `never` and cancels concurrent test runs on the first matching result. If enabled, it's now effective by default and no longer requires `--test_strategy=standalone` to be passed explicitly.
Closes #26630.
PiperOrigin-RevId: 789203343
Change-Id: I5e7838fa3562a1137987efbf2235d22b122e3e51
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
index 2ccd1b8..6592dd7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.analysis.actions.LazyWriteNestedSetOfTupleAction;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
+import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions.CancelConcurrentTests;
import com.google.devtools.build.lib.analysis.test.TestProvider.TestParams;
import com.google.devtools.build.lib.analysis.test.TestProvider.TestParams.CoverageParams;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -403,9 +404,10 @@
Artifact undeclaredOutputsDir =
ruleContext.getPackageRelativeTreeArtifact(dir.getRelative("test.outputs"), root);
- boolean cancelConcurrentTests =
+ CancelConcurrentTests cancelConcurrentTests =
testConfiguration.runsPerTestDetectsFlakes()
- && testConfiguration.cancelConcurrentTests();
+ ? testConfiguration.cancelConcurrentTests()
+ : CancelConcurrentTests.NEVER;
boolean splitCoveragePostProcessing = testConfiguration.splitCoveragePostProcessing();
TestRunnerAction testRunnerAction =
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java
index 1f1b807..be74ea8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionContext.java
@@ -28,18 +28,17 @@
import java.util.List;
import javax.annotation.Nullable;
-/**
- * A context for the execution of test actions ({@link TestRunnerAction}).
- */
+/** A context for the execution of test actions ({@link TestRunnerAction}). */
public interface TestActionContext extends ActionContext {
/**
* A group of attempts for a single test shard, ran either sequentially or in parallel.
*
- * <p>When one attempt succeeds, threads running the other attempts get an {@link
- * InterruptedException} and {@link #cancelled()} will in the future return true. When a thread
- * joins an attempt group that is already cancelled, {@link InterruptedException} will be thrown
- * on the call to {@link #register()}.
+ * <p>When one attempt matches the result specified by {@link
+ * com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions.CancelConcurrentTests},
+ * threads running the other attempts get an {@link InterruptedException} and {@link #cancelled()}
+ * will in the future return true. When a thread joins an attempt group that is already cancelled,
+ * {@link InterruptedException} will be thrown on the call to {@link #register()}.
*/
interface AttemptGroup {
@@ -53,7 +52,9 @@
/** Unregisters a thread from the attempt group. */
void unregister();
- /** Signal that the attempt run by this thread has succeeded and cancel all the others. */
+ /**
+ * Signal that the attempt run by this thread has the desired result and cancel all the others.
+ */
void cancelOthers();
/** Whether the attempt group has been cancelled. */
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java
index 401e3ba..ea8aab1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java
@@ -28,12 +28,14 @@
import com.google.devtools.build.lib.analysis.config.RequiresOptions;
import com.google.devtools.build.lib.analysis.config.RunUnder;
import com.google.devtools.build.lib.analysis.test.CoverageConfiguration.CoverageOptions;
+import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions.CancelConcurrentTests;
import com.google.devtools.build.lib.analysis.test.TestShardingStrategy.ShardingStrategyConverter;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.TestSize;
import com.google.devtools.build.lib.packages.TestTimeout;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.RegexFilter;
+import com.google.devtools.common.options.BoolOrEnumConverter;
import com.google.devtools.common.options.Converters;
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDefinition;
@@ -269,16 +271,32 @@
+ "run/attempt fails gets a FLAKY status.")
public boolean runsPerTestDetectsFlakes;
+ /** When to cancel concurrently running tests. */
+ public enum CancelConcurrentTests {
+ NEVER,
+ ON_FAILED,
+ ON_PASSED;
+
+ /** Converts to {@link CancelConcurrentTests}. */
+ static class Converter extends BoolOrEnumConverter<CancelConcurrentTests> {
+ public Converter() {
+ super(CancelConcurrentTests.class, "when to cancel concurrent tests", ON_PASSED, NEVER);
+ }
+ }
+ }
+
@Option(
name = "experimental_cancel_concurrent_tests",
- defaultValue = "false",
+ defaultValue = "never",
+ converter = CancelConcurrentTests.Converter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS, OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
help =
- "If true, then Blaze will cancel concurrently running tests on the first successful "
- + "run. This is only useful in combination with --runs_per_test_detects_flakes.")
- public boolean cancelConcurrentTests;
+ "If 'on_failed' or 'on_passed, then Blaze will cancel concurrently running tests on "
+ + "the first run with that result. This is only useful in combination with "
+ + "--runs_per_test_detects_flakes.")
+ public CancelConcurrentTests cancelConcurrentTests;
@Option(
name = "coverage_support",
@@ -446,7 +464,7 @@
return options.runsPerTestDetectsFlakes;
}
- public boolean cancelConcurrentTests() {
+ public CancelConcurrentTests cancelConcurrentTests() {
return options.cancelConcurrentTests;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
index 24ec8e8..62786d2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
@@ -55,6 +55,7 @@
import com.google.devtools.build.lib.analysis.test.TestActionContext.TestAttemptResult;
import com.google.devtools.build.lib.analysis.test.TestActionContext.TestAttemptResult.Result;
import com.google.devtools.build.lib.analysis.test.TestActionContext.TestRunnerSpawn;
+import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions.CancelConcurrentTests;
import com.google.devtools.build.lib.buildeventstream.TestFileNameConstants;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -163,7 +164,7 @@
*/
private final Collection<String> requiredClientEnvVariables;
- private final boolean cancelConcurrentTestsOnSuccess;
+ private final CancelConcurrentTests cancelConcurrentTests;
private final boolean splitCoveragePostProcessing;
private final NestedSetBuilder<Artifact> lcovMergerFilesToRun;
@@ -210,7 +211,7 @@
BuildConfigurationValue configuration,
String workspaceName,
@Nullable PathFragment shExecutable,
- boolean cancelConcurrentTestsOnSuccess,
+ CancelConcurrentTests cancelConcurrentTests,
boolean splitCoveragePostProcessing,
NestedSetBuilder<Artifact> lcovMergerFilesToRun,
@Nullable Artifact lcovMergerRunfilesTree,
@@ -268,7 +269,7 @@
configuration.getActionEnvironment().getInheritedEnv(),
configuration.getTestActionEnvironment().getInheritedEnv(),
this.extraTestEnv.getInheritedEnv());
- this.cancelConcurrentTestsOnSuccess = cancelConcurrentTestsOnSuccess;
+ this.cancelConcurrentTests = cancelConcurrentTests;
this.splitCoveragePostProcessing = splitCoveragePostProcessing;
this.lcovMergerFilesToRun = lcovMergerFilesToRun;
this.lcovMergerRunfilesTree = lcovMergerRunfilesTree;
@@ -1011,15 +1012,22 @@
try {
testRunnerSpawn = testActionContext.createTestRunnerSpawn(this, actionExecutionContext);
attemptGroup =
- cancelConcurrentTestsOnSuccess
+ cancelConcurrentTests != CancelConcurrentTests.NEVER
? testActionContext.getAttemptGroup(getOwner(), shardNum)
: AttemptGroup.NOOP;
+ var cancelOnResult =
+ switch (cancelConcurrentTests) {
+ case NEVER -> null;
+ case ON_FAILED -> Result.FAILED_CAN_RETRY;
+ case ON_PASSED -> Result.PASSED;
+ };
try {
attemptGroup.register();
var result =
executeAllAttempts(
testRunnerSpawn,
testActionContext.isTestKeepGoing(),
+ cancelOnResult,
attemptGroup,
spawnResults,
failedAttempts);
@@ -1198,6 +1206,7 @@
public ActionResult executeAllAttempts(
TestRunnerSpawn testRunnerSpawn,
boolean keepGoing,
+ @Nullable Result cancelOnResult,
final AttemptGroup attemptGroup,
List<SpawnResult> spawnResults,
List<ProcessedAttemptResult> failedAttempts)
@@ -1212,9 +1221,10 @@
spawnResults.addAll(result.spawnResults());
TestAttemptResult.Result testResult = result.result();
- if (testResult == TestAttemptResult.Result.PASSED) {
+ if (testResult == cancelOnResult) {
attemptGroup.cancelOthers();
- } else {
+ }
+ if (testResult != TestAttemptResult.Result.PASSED) {
TestRunnerSpawnAndMaxAttempts nextRunnerAndAttempts =
computeNextRunnerAndMaxAttempts(
testResult,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java
index 2ab9025..1bc4610 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/ExclusiveTestStrategy.java
@@ -31,9 +31,13 @@
* <p>This strategy should be registered with a command line identifier of 'exclusive' which will
* trigger behavior in SkyframeExecutor to schedule test execution sequentially after non-test
* actions. This ensures streamed test output is not polluted by other action output.
+ *
+ * <p>Note: It's expected that this strategy is largely identical to the one it wraps. Most of the
+ * behavior specific to the 'exclusive' strategy is enabled based on the value of the <code>
+ * --test_strategy</code> flag, not instance methods of this class.
*/
public class ExclusiveTestStrategy implements TestActionContext {
- private TestActionContext parent;
+ private final TestActionContext parent;
public ExclusiveTestStrategy(TestActionContext parent) {
this.parent = parent;
diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
index c3ba42e..323cf07 100644
--- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
+++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneModule.java
@@ -62,9 +62,10 @@
TestStrategy.getTmpRoot(env.getWorkspace(), env.getExecRoot(), executionOptions);
TestActionContext testStrategy =
new StandaloneTestStrategy(executionOptions, testSummaryOptions, testTmpRoot);
- registryBuilder.register(TestActionContext.class, testStrategy, "standalone");
+ // Keep the standalone test strategy last so that it is the default one.
registryBuilder.register(
TestActionContext.class, new ExclusiveTestStrategy(testStrategy), "exclusive");
+ registryBuilder.register(TestActionContext.class, testStrategy, "standalone");
registryBuilder.register(FileWriteActionContext.class, new FileWriteStrategy(), "local");
registryBuilder.register(
TemplateExpansionContext.class, new LocalTemplateExpansionStrategy(), "local");
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 170da45..b44d141 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD
@@ -37,6 +37,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
+ "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis/config:build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis/config:build_options",
"//src/main/java/com/google/devtools/build/lib/analysis/config:core_options",
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 4c7dde5..4425e25 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
@@ -47,6 +47,7 @@
import com.google.devtools.build.lib.analysis.test.TestActionContext.ProcessedAttemptResult;
import com.google.devtools.build.lib.analysis.test.TestActionContext.TestRunnerSpawn;
import com.google.devtools.build.lib.analysis.test.TestAttempt;
+import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions.CancelConcurrentTests;
import com.google.devtools.build.lib.analysis.test.TestProvider;
import com.google.devtools.build.lib.analysis.test.TestResult;
import com.google.devtools.build.lib.analysis.test.TestRunnerAction;
@@ -76,6 +77,8 @@
import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus;
import com.google.devtools.build.lib.view.test.TestStatus.TestResultData;
import com.google.devtools.common.options.Options;
+import com.google.testing.junit.testparameterinjector.TestParameter;
+import com.google.testing.junit.testparameterinjector.TestParameterInjector;
import java.io.IOException;
import java.io.OutputStream;
import java.util.ArrayList;
@@ -86,18 +89,26 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
/** Unit tests for {@link StandaloneTestStrategy}. */
-@RunWith(JUnit4.class)
+@RunWith(TestParameterInjector.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 final SpawnResult FAILED_TEST_SPAWN =
+ new SpawnResult.Builder()
+ .setStatus(Status.NON_ZERO_EXIT)
+ .setExitCode(1)
+ .setFailureDetail(NON_ZERO_EXIT_DETAILS)
+ .setRunnerName("test")
+ .build();
+ private static final SpawnResult PASSED_TEST_SPAWN =
+ new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
private static class TestedStandaloneTestStrategy extends StandaloneTestStrategy {
TestResult postedResult = null;
@@ -564,13 +575,7 @@
""");
TestRunnerAction testRunnerAction = getTestAction("//standalone:failing_test");
- SpawnResult expectedSpawnResult =
- new SpawnResult.Builder()
- .setStatus(Status.NON_ZERO_EXIT)
- .setExitCode(1)
- .setFailureDetail(NON_ZERO_EXIT_DETAILS)
- .setRunnerName("test")
- .build();
+ SpawnResult expectedSpawnResult = FAILED_TEST_SPAWN;
when(spawnStrategy.exec(any(), any()))
.thenAnswer(
(invocation) -> {
@@ -589,11 +594,7 @@
/* forciblyRunRemotely= */ false,
/* catastrophe= */ false);
} else {
- return ImmutableList.of(
- new SpawnResult.Builder()
- .setStatus(Status.SUCCESS)
- .setRunnerName("test")
- .build());
+ return ImmutableList.of(PASSED_TEST_SPAWN);
}
});
@@ -650,15 +651,8 @@
""");
TestRunnerAction testRunnerAction = getTestAction("//standalone:failing_test");
- SpawnResult testSpawnResult =
- new SpawnResult.Builder()
- .setStatus(Status.NON_ZERO_EXIT)
- .setExitCode(1)
- .setFailureDetail(NON_ZERO_EXIT_DETAILS)
- .setRunnerName("test")
- .build();
- SpawnResult xmlGeneratorSpawnResult =
- new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
+ SpawnResult testSpawnResult = FAILED_TEST_SPAWN;
+ SpawnResult xmlGeneratorSpawnResult = PASSED_TEST_SPAWN;
List<FileOutErr> called = new ArrayList<>();
when(spawnStrategy.exec(any(), any()))
.thenAnswer(
@@ -740,8 +734,7 @@
""");
TestRunnerAction testRunnerAction = getTestAction("//standalone:empty_test");
- SpawnResult expectedSpawnResult =
- new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
+ SpawnResult expectedSpawnResult = PASSED_TEST_SPAWN;
when(spawnStrategy.exec(any(), any())).thenReturn(ImmutableList.of(expectedSpawnResult));
FileOutErr outErr = createTempOutErr(tmpDirRoot);
@@ -790,13 +783,11 @@
""");
TestRunnerAction testRunnerAction = getTestAction("//standalone:empty_test");
- SpawnResult expectedSpawnResult =
- new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
when(spawnStrategy.exec(any(), any()))
.then(
(invocation) -> {
((ActionExecutionContext) invocation.getArgument(1)).getFileOutErr().printErr("Foo");
- return ImmutableList.of(expectedSpawnResult);
+ return ImmutableList.of(PASSED_TEST_SPAWN);
});
FileOutErr outErr = createTempOutErr(tmpDirRoot);
@@ -812,11 +803,14 @@
}
@Test
- public void testExperimentalCancelConcurrentTests() throws Exception {
+ public void testExperimentalCancelConcurrentTests(
+ @TestParameter({"ON_PASSED", "ON_FAILED"}) CancelConcurrentTests cancelConcurrentTests)
+ throws Exception {
useConfiguration(
"--runs_per_test=2",
"--runs_per_test_detects_flakes",
- "--experimental_cancel_concurrent_tests");
+ "--experimental_cancel_concurrent_tests=" + cancelConcurrentTests);
+ boolean testOnPassed = cancelConcurrentTests == CancelConcurrentTests.ON_PASSED;
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
@@ -845,15 +839,18 @@
.isSameInstanceAs(
standaloneTestStrategy.getAttemptGroup(actionB.getOwner(), actionB.getShardNum()));
- SpawnResult expectedSpawnResult =
- new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
when(spawnStrategy.exec(any(), any()))
.then(
(invocation) -> {
// Avoid triggering split XML generation by creating an empty XML file.
FileSystemUtils.touchFile(actionA.resolve(getExecRoot()).getXmlOutputPath());
- return ImmutableList.of(expectedSpawnResult);
- });
+ if (testOnPassed) {
+ return ImmutableList.of(PASSED_TEST_SPAWN);
+ } else {
+ throw new SpawnExecException("", FAILED_TEST_SPAWN, false);
+ }
+ })
+ .thenThrow(new AssertionError("failure: this should not have been called"));
FakeActionInputFileCache inputMetadataProvider = new FakeActionInputFileCache();
inputMetadataProvider.putRunfilesTree(actionA.getRunfilesTree(), runfilesTreeFor(actionA));
@@ -869,15 +866,18 @@
assertThat(resultA).hasSize(1);
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)"));
+ .isEqualTo(testOnPassed ? BlazeTestStatus.PASSED : BlazeTestStatus.FAILED);
+ assertThat(standaloneTestStrategy.postedResult.getData().getExitCode())
+ .isEqualTo(testOnPassed ? 0 : 1);
+ assertContainsPrefixedEvent(
+ storedEvents.getEvents(),
+ Event.of(
+ testOnPassed ? EventKind.PASS : EventKind.FAIL,
+ null,
+ "//standalone:empty_test (run 1 of 2)"));
// Reset postedResult.
standaloneTestStrategy.postedResult = null;
- when(spawnStrategy.exec(any(), any()))
- .thenThrow(new AssertionError("failure: this should not have been called"));
ImmutableList<SpawnResult> resultB =
execute(actionB, actionExecutionContext, standaloneTestStrategy);
assertThat(resultB).isEmpty();
@@ -895,11 +895,14 @@
}
@Test
- public void testExperimentalCancelConcurrentTestsDoesNotTriggerOnFailedRun() throws Exception {
+ public void testExperimentalCancelConcurrentTestsDoesNotTriggerOnUnexpectedResult(
+ @TestParameter({"ON_PASSED", "ON_FAILED"}) CancelConcurrentTests cancelConcurrentTests)
+ throws Exception {
useConfiguration(
"--runs_per_test=2",
"--runs_per_test_detects_flakes",
- "--experimental_cancel_concurrent_tests");
+ "--experimental_cancel_concurrent_tests=" + cancelConcurrentTests);
+ boolean testOnPassed = cancelConcurrentTests == CancelConcurrentTests.ON_PASSED;
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
@@ -929,28 +932,26 @@
standaloneTestStrategy.getAttemptGroup(actionB.getOwner(), actionB.getShardNum()));
assertThat(attemptGroup.cancelled()).isFalse();
- SpawnResult expectedSpawnResultA =
- new SpawnResult.Builder()
- .setStatus(Status.NON_ZERO_EXIT)
- .setExitCode(1)
- .setFailureDetail(NON_ZERO_EXIT_DETAILS)
- .setRunnerName("test")
- .build();
- SpawnResult expectedSpawnResultB =
- new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
-
when(spawnStrategy.exec(any(), any()))
.then(
(invocation) -> {
// Avoid triggering split XML generation by creating an empty XML file.
FileSystemUtils.touchFile(actionA.resolve(getExecRoot()).getXmlOutputPath());
- throw new SpawnExecException("", expectedSpawnResultA, false);
+ if (testOnPassed) {
+ throw new SpawnExecException("", FAILED_TEST_SPAWN, false);
+ } else {
+ return ImmutableList.of(PASSED_TEST_SPAWN);
+ }
})
.then(
(invocation) -> {
// Avoid triggering split XML generation by creating an empty XML file.
FileSystemUtils.touchFile(actionB.resolve(getExecRoot()).getXmlOutputPath());
- return ImmutableList.of(expectedSpawnResultB);
+ if (testOnPassed) {
+ return ImmutableList.of(PASSED_TEST_SPAWN);
+ } else {
+ throw new SpawnExecException("", FAILED_TEST_SPAWN, false);
+ }
});
FakeActionInputFileCache inputMetadataProvider = new FakeActionInputFileCache();
@@ -967,31 +968,33 @@
assertThat(resultA).hasSize(1);
assertThat(standaloneTestStrategy.postedResult).isNotNull();
assertThat(standaloneTestStrategy.postedResult.getData().getStatus())
- .isEqualTo(BlazeTestStatus.FAILED);
- assertThat(standaloneTestStrategy.postedResult.getData().getExitCode()).isEqualTo(1);
+ .isEqualTo(testOnPassed ? BlazeTestStatus.FAILED : BlazeTestStatus.PASSED);
+ assertThat(standaloneTestStrategy.postedResult.getData().getExitCode())
+ .isEqualTo(testOnPassed ? 1 : 0);
assertContainsPrefixedEvent(
storedEvents.getEvents(),
- Event.of(EventKind.FAIL, null, "//standalone:empty_test (run 1 of 2)"));
+ Event.of(
+ testOnPassed ? EventKind.FAIL : EventKind.PASS,
+ null,
+ "//standalone:empty_test (run 1 of 2)"));
// Reset postedResult.
standaloneTestStrategy.postedResult = null;
- when(spawnStrategy.exec(any(), any()))
- .then(
- (invocation) -> {
- // Avoid triggering split XML generation by creating an empty XML file.
- FileSystemUtils.touchFile(actionB.resolve(getExecRoot()).getXmlOutputPath());
- return ImmutableList.of(expectedSpawnResultB);
- });
ImmutableList<SpawnResult> resultB =
execute(actionB, actionExecutionContext, standaloneTestStrategy);
assertThat(attemptGroup.cancelled()).isTrue();
assertThat(resultB).hasSize(1);
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)"));
+ .isEqualTo(testOnPassed ? BlazeTestStatus.PASSED : BlazeTestStatus.FAILED);
+ assertThat(standaloneTestStrategy.postedResult.getData().getExitCode())
+ .isEqualTo(testOnPassed ? 0 : 1);
+ assertContainsPrefixedEvent(
+ storedEvents.getEvents(),
+ Event.of(
+ testOnPassed ? EventKind.PASS : EventKind.FAIL,
+ null,
+ "//standalone:empty_test (run 2 of 2)"));
}
private static void assertContainsPrefixedEvent(Iterable<Event> events, Event event) {
@@ -1004,11 +1007,14 @@
}
@Test
- public void testExperimentalCancelConcurrentTestsAllFailed() throws Exception {
+ public void testExperimentalCancelConcurrentTestsAllUnexpected(
+ @TestParameter({"ON_PASSED", "ON_FAILED"}) CancelConcurrentTests cancelConcurrentTests)
+ throws Exception {
useConfiguration(
"--runs_per_test=2",
"--runs_per_test_detects_flakes",
- "--experimental_cancel_concurrent_tests");
+ "--experimental_cancel_concurrent_tests=" + cancelConcurrentTests);
+ boolean testOnPassed = cancelConcurrentTests == CancelConcurrentTests.ON_PASSED;
ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class);
Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
@@ -1038,25 +1044,26 @@
standaloneTestStrategy.getAttemptGroup(actionB.getOwner(), actionB.getShardNum()));
assertThat(attemptGroup.cancelled()).isFalse();
- SpawnResult expectedSpawnResult =
- new SpawnResult.Builder()
- .setStatus(Status.NON_ZERO_EXIT)
- .setExitCode(1)
- .setFailureDetail(NON_ZERO_EXIT_DETAILS)
- .setRunnerName("test")
- .build();
when(spawnStrategy.exec(any(), any()))
.then(
(invocation) -> {
// Avoid triggering split XML generation by creating an empty XML file.
FileSystemUtils.touchFile(actionA.resolve(getExecRoot()).getXmlOutputPath());
- throw new SpawnExecException("", expectedSpawnResult, false);
+ if (testOnPassed) {
+ throw new SpawnExecException("", FAILED_TEST_SPAWN, false);
+ } else {
+ return ImmutableList.of(PASSED_TEST_SPAWN);
+ }
})
.then(
(invocation) -> {
// Avoid triggering split XML generation by creating an empty XML file.
FileSystemUtils.touchFile(actionB.resolve(getExecRoot()).getXmlOutputPath());
- throw new SpawnExecException("", expectedSpawnResult, false);
+ if (testOnPassed) {
+ throw new SpawnExecException("", FAILED_TEST_SPAWN, false);
+ } else {
+ return ImmutableList.of(PASSED_TEST_SPAWN);
+ }
});
FakeActionInputFileCache inputMetadataProvider = new FakeActionInputFileCache();
@@ -1073,11 +1080,15 @@
assertThat(resultA).hasSize(1);
assertThat(standaloneTestStrategy.postedResult).isNotNull();
assertThat(standaloneTestStrategy.postedResult.getData().getStatus())
- .isEqualTo(BlazeTestStatus.FAILED);
- assertThat(standaloneTestStrategy.postedResult.getData().getExitCode()).isEqualTo(1);
+ .isEqualTo(testOnPassed ? BlazeTestStatus.FAILED : BlazeTestStatus.PASSED);
+ assertThat(standaloneTestStrategy.postedResult.getData().getExitCode())
+ .isEqualTo(testOnPassed ? 1 : 0);
assertContainsPrefixedEvent(
storedEvents.getEvents(),
- Event.of(EventKind.FAIL, null, "//standalone:empty_test (run 1 of 2)"));
+ Event.of(
+ testOnPassed ? EventKind.FAIL : EventKind.PASS,
+ null,
+ "//standalone:empty_test (run 1 of 2)"));
// Reset postedResult.
standaloneTestStrategy.postedResult = null;
@@ -1087,10 +1098,13 @@
assertThat(resultB).hasSize(1);
assertThat(standaloneTestStrategy.postedResult).isNotNull();
assertThat(standaloneTestStrategy.postedResult.getData().getStatus())
- .isEqualTo(BlazeTestStatus.FAILED);
+ .isEqualTo(testOnPassed ? BlazeTestStatus.FAILED : BlazeTestStatus.PASSED);
assertContainsPrefixedEvent(
storedEvents.getEvents(),
- Event.of(EventKind.FAIL, null, "//standalone:empty_test (run 2 of 2)"));
+ Event.of(
+ testOnPassed ? EventKind.FAIL : EventKind.PASS,
+ null,
+ "//standalone:empty_test (run 2 of 2)"));
}
@Test
diff --git a/src/test/shell/bazel/bazel_test_test.sh b/src/test/shell/bazel/bazel_test_test.sh
index 845eb7f..0b0e74c 100755
--- a/src/test/shell/bazel/bazel_test_test.sh
+++ b/src/test/shell/bazel/bazel_test_test.sh
@@ -475,10 +475,48 @@
--runs_per_test=5 \
--runs_per_test_detects_flakes \
//:test$i &> $TEST_log || fail "should have succeeded"
- expect_log "FLAKY"
+ expect_log "FLAKY, failed in 4 out of 5"
done
}
+function test_runs_per_test_detects_flakes_cancel_concurrent() {
+ # Directory for counters
+ local COUNTER_DIR="${TEST_TMPDIR}/counter_dir"
+ mkdir -p "${COUNTER_DIR}"
+ add_rules_shell "MODULE.bazel"
+
+ # This file holds the number of the next run
+ echo 1 > "${COUNTER_DIR}/counter"
+ cat <<EOF > test.sh
+#!/bin/sh
+i=\$(cat "${COUNTER_DIR}/counter")
+
+echo "Run \$i"
+
+# increment the hidden state
+echo \$((i + 1)) > "${COUNTER_DIR}/counter"
+
+# succeed in the first two runs, fail in the third one
+exit \$((i <= 2 ? 0 : 1))
+}
+EOF
+ chmod +x test.sh
+ cat <<EOF > BUILD
+load("@rules_shell//shell:sh_test.bzl", "sh_test")
+
+sh_test(name = "test", srcs = [ "test.sh" ])
+EOF
+ bazel test --spawn_strategy=standalone \
+ --jobs=1 \
+ --runs_per_test=5 \
+ --runs_per_test_detects_flakes \
+ --experimental_cancel_concurrent_tests=on_failed \
+ //:test &> $TEST_log || fail "should have succeeded"
+ expect_log_n "^FAIL: //:test" 1
+ expect_log_n "^CANCELLED: //:test" 2
+ expect_log "FLAKY, failed in 1 out of 3"
+}
+
# Tests that the test.xml is extracted from the sandbox correctly.
function test_xml_is_present() {
mkdir -p dir
