Repository: bazelbuild/bazel
Issue: pass platform and execution requirements to xml generating spawn
Labels: P1
team-Remote-Exec

The fix for https://github.com/bazelbuild/bazel/issues/4608 splits a test action into two spawns. The first spawn runs the actual test and the second spawn declares the `test.log` as an input and produces a test.xml file. 

We found an issue where a `sh_test` target was marked as `no-remote` but the test.xml-generating spawn would not get this tag passed along and would thus run remotely.

The proposed fix is to pass along the execution requirements and platform of the test spawn to the test.xml-generating spawn.

Issue comments:
Does this issue only happen when `test.log` is very large?
no it's independent of the test.log size.
The workaround for this issue is to use `--experimental_split_xml_generation=false`

Pull request review comments:

Patch that resolved the issue:
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 43e72787d289c8..cc483672c9d046 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
@@ -465,7 +465,9 @@ private Spawn createXmlGeneratingSpawn(TestRunnerAction action, SpawnResult resu
             "TEST_TOTAL_SHARDS", Integer.toString(action.getExecutionSettings().getTotalShards()),
             "TEST_NAME", action.getTestName(),
             "TEST_BINARY", testBinaryName),
-        ImmutableMap.of(),
+        // Pass the execution info of the action which is identical to the supported tags set on the
+        // test target. In particular, this does not set the test timeout on the spawn.
+        ImmutableMap.copyOf(action.getExecutionInfo()),
         null,
         ImmutableMap.of(),
         /*inputs=*/ ImmutableList.of(action.getTestXmlGeneratorScript(), action.getTestLog()),


Test patch:
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 771958614fb948..219d89e041e650 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
@@ -519,6 +519,7 @@ public void testThatTestLogAndOutputAreReturnedWithSplitXmlGeneration() throws E
         "    name = \"failing_test\",",
         "    size = \"small\",",
         "    srcs = [\"failing_test.sh\"],",
+        "    tags = [\"local\"],",
         ")");
     TestRunnerAction testRunnerAction = getTestAction("//standalone:failing_test");
 
@@ -538,6 +539,8 @@ public void testThatTestLogAndOutputAreReturnedWithSplitXmlGeneration() throws E
         .thenAnswer(
             (invocation) -> {
               Spawn spawn = invocation.getArgument(0);
+              // Test that both spawns have the local tag attached as a execution info
+              assertThat(spawn.getExecutionInfo()).containsKey("local");
               ActionExecutionContext context = invocation.getArgument(1);
               FileOutErr outErr = context.getFileOutErr();
               called.add(outErr);