Actually check TEST_SHARD_STATUS_FILE has been touched (#18418)

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

Co-authored-by: keertk <[email protected]>
Co-authored-by: Yun Peng <[email protected]>

site/en/reference/test-encyclopedia.md

@@ -223,8 +223,9 @@ index, beginning at 0. Runners use this information to select which tests to
 run - for example, using a round-robin strategy. Not all test runners support
 sharding. If a runner supports sharding, it must create or update the last
 modified date of the file specified by
-[`TEST_SHARD_STATUS_FILE`](#initial-conditions). Otherwise, Bazel assumes it
-does not support sharding and will not launch additional runners.
+[`TEST_SHARD_STATUS_FILE`](#initial-conditions). Otherwise, if
+[`--incompatible_check_sharding_support`](/reference/command-line-reference#flag--incompatible_check_sharding_support)
+is enabled, Bazel will fail the test if it is sharded.
 
 ## Initial conditions {:#initial-conditions}
 

src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java

@@ -280,6 +280,19 @@ public static class TestOptions extends FragmentOptions {
         help = "If true, undeclared test outputs will be archived in a zip file.")
     public boolean zipUndeclaredTestOutputs;
 
+    @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 getHost() {
       // Options here are either:
@@ -380,6 +393,10 @@ public boolean getZipUndeclaredTestOutputs() {
     return options.zipUndeclaredTestOutputs;
   }
 
+  public boolean checkShardingSupport() {
+    return options.checkShardingSupport;
+  }
+
   /**
    * Option converter that han handle two styles of value for "--runs_per_test":
    *

src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java

@@ -336,6 +336,10 @@ public boolean showsOutputUnconditionally() {
     return true;
   }
 
+  public boolean checkShardingSupport() {
+    return testConfiguration.checkShardingSupport();
+  }
+
   public List<ActionInput> getSpawnOutputs() {
     final List<ActionInput> outputs = new ArrayList<>();
     outputs.add(ActionInputHelper.fromPath(getXmlOutputPath()));

src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java

@@ -747,6 +747,25 @@ public TestAttemptContinuation execute()
         }
         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);

src/test/py/bazel/bazel_windows_test.py

@@ -451,6 +451,41 @@ def testZipUndeclaredTestOutputs(self):
     self.assertTrue(os.path.exists(output_file))
     self.assertFalse(os.path.exists(output_zip))
 
+  def testTestShardStatusFile(self):
+    self.CreateWorkspaceWithDefaultRepos('WORKSPACE')
+    self.ScratchFile(
+        'BUILD',
+        [
+            'sh_test(',
+            '  name = "foo_test",',
+            '  srcs = ["foo.sh"],',
+            '  shard_count = 2,',
+            ')',
+        ],
+    )
+    self.ScratchFile('foo.sh')
+
+    exit_code, stdout, stderr = self.RunBazel(
+        ['test', '--incompatible_check_sharding_support', '//:foo_test'],
+        allow_failure=True,
+    )
+    # Check for "tests failed" exit code
+    self.AssertExitCode(exit_code, 3, stderr, stdout)
+    self.assertTrue(
+        any(
+            'Sharding requested, but the test runner did not advertise support'
+            ' for it by touching TEST_SHARD_STATUS_FILE.'
+            in line
+            for line in stderr
+        )
+    )
+
+    self.ScratchFile('foo.sh', ['touch "$TEST_SHARD_STATUS_FILE"'])
+
+    self.RunBazel(
+        ['test', '--incompatible_check_sharding_support', '//:foo_test']
+    )
+
 
 if __name__ == '__main__':
   unittest.main()

src/test/shell/bazel/bazel_test_test.sh

@@ -997,4 +997,26 @@ EOF
   expect_log "<testcase name=\"x\""
 }
 
+function test_shard_status_file_checked() {
+  cat <<'EOF' > BUILD
+sh_test(
+    name = 'x',
+    srcs = ['x.sh'],
+    shard_count = 2,
+)
+EOF
+  touch x.sh
+  chmod +x x.sh
+
+  bazel test \
+      --incompatible_check_sharding_support \
+      //:x  &> $TEST_log && fail "expected failure"
+  expect_log "Sharding requested, but the test runner did not advertise support for it by touching TEST_SHARD_STATUS_FILE."
+
+  echo 'touch "$TEST_SHARD_STATUS_FILE"' > x.sh
+  bazel test \
+      --incompatible_check_sharding_support \
+      //:x  &> $TEST_log || fail "expected success"
+}
+
 run_suite "bazel test tests"

src/test/shell/bazel/remote/remote_execution_test.sh

@@ -3250,4 +3250,30 @@ function test_external_cc_binary_tool_with_dynamic_deps_sibling_repository_layou
       @other_repo//pkg:rule >& $TEST_log || fail "Build should succeed"
 }
 
+function test_shard_status_file_checked_remote_download_minimal() {
+  cat <<'EOF' > BUILD
+sh_test(
+    name = 'x',
+    srcs = ['x.sh'],
+    shard_count = 2,
+)
+EOF
+  touch x.sh
+  chmod +x x.sh
+
+  bazel test \
+      --remote_executor=grpc://localhost:${worker_port} \
+      --incompatible_check_sharding_support \
+      --remote_download_minimal \
+      //:x  &> $TEST_log && fail "expected failure"
+  expect_log "Sharding requested, but the test runner did not advertise support for it by touching TEST_SHARD_STATUS_FILE."
+
+  echo 'touch "$TEST_SHARD_STATUS_FILE"' > x.sh
+  bazel test \
+      --remote_executor=grpc://localhost:${worker_port} \
+      --incompatible_check_sharding_support \
+      --remote_download_minimal \
+      //:x  &> $TEST_log || fail "expected success"
+}
+
 run_suite "Remote execution and remote cache tests"