Actually check `TEST_SHARD_STATUS_FILE` has been touched
Adds the missing check that a test runner running a sharded test actually declares support for sharding by generating the `TEST_SHARD_STATUS_FILE`. Previously, if it didn't, each shard would silently run all tests. This new behavior is guarded by the new `--incompatible_check_sharding_support` flag.
Closes #18236.
PiperOrigin-RevId: 530259354
Change-Id: If3b01b2c796e66ad7a77e542145efe3ab412eae9
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 ff92580..333dccc 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
@@ -291,6 +291,19 @@
+ "the test exec group.")
public boolean useTargetPlatformForTests;
+ @Option(
+ name = "incompatible_check_sharding_support",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+ metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+ effectTags = {OptionEffectTag.UNKNOWN},
+ help =
+ "If true, Bazel will fail a sharded test if the test runner does not indicate that it "
+ + "supports sharding by touching the file at the path in TEST_SHARD_STATUS_FILE. "
+ + "If false, a test runner that does not support sharding will lead to all tests "
+ + "running in each shard.")
+ public boolean checkShardingSupport;
+
@Override
public FragmentOptions getExec() {
// Options here are either:
@@ -395,6 +408,10 @@
return options.useTargetPlatformForTests;
}
+ public boolean checkShardingSupport() {
+ return options.checkShardingSupport;
+ }
+
/**
* Option converter that han handle two styles of value for "--runs_per_test":
*
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 8ead7e0..512caef 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
@@ -332,6 +332,10 @@
return true;
}
+ public boolean checkShardingSupport() {
+ return testConfiguration.checkShardingSupport();
+ }
+
public List<ActionInput> getSpawnOutputs() {
final List<ActionInput> outputs = new ArrayList<>();
outputs.add(ActionInputHelper.fromPath(getXmlOutputPath()));
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 2afb45f..30bd217 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
@@ -684,6 +684,25 @@
}
long endTimeMillis = actionExecutionContext.getClock().currentTimeMillis();
+ if (testAction.isSharded()) {
+ if (testAction.checkShardingSupport()
+ && !actionExecutionContext
+ .getPathResolver()
+ .convertPath(resolvedPaths.getTestShard())
+ .exists()) {
+ TestExecException e =
+ createTestExecException(
+ TestAction.Code.LOCAL_TEST_PREREQ_UNMET,
+ "Sharding requested, but the test runner did not advertise support for it by "
+ + "touching TEST_SHARD_STATUS_FILE. Either remove the 'shard_count' attribute, "
+ + "use a test runner that supports sharding or temporarily disable this check "
+ + "via --noincompatible_check_sharding_support.");
+ closeSuppressed(e, streamed);
+ closeSuppressed(e, fileOutErr);
+ throw e;
+ }
+ }
+
// SpawnActionContext guarantees the first entry to correspond to the spawn passed in (there
// may be additional entries due to tree artifact handling).
SpawnResult primaryResult = spawnResults.get(0);
