Fix StandaloneTestStrategy.appendStderr

As of 4a5e1b7, it was using getErrorPath twice, which could cause it to
loop indefinitely, trying to append the error file to itself. This
happened rarely, as the test runner script redirects stderr to stdout.
However, it could happen if the SpawnRunner wrote any extra output to
stderr, which the RemoteSpawnRunner does in some cases.

I have manually checked that this fixes the issue, and also added a
regression test.

Fixes bazelbuild#8320.

PiperOrigin-RevId: 249258656

Author: ulfjack

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

@@ -323,7 +323,7 @@ private static void appendStderr(FileOutErr outErr) throws IOException {
     if (stat != null) {
       try {
         if (stat.getSize() > 0) {
-          Path stdOut = outErr.getErrorPath();
+          Path stdOut = outErr.getOutputPath();
           if (stdOut.exists()) {
             stdOut.setWritable(true);
           }

src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java

@@ -655,4 +655,49 @@ public void testEmptyOutputCreatesEmptyLogFile() throws Exception {
     }
     assertThat(outErr.getErrorPath().exists()).isFalse();
   }
+
+  @Test
+  public void testAppendStdErrDoesNotBusyLoop() throws Exception {
+    ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class);
+    executionOptions.testOutput = TestOutputFormat.ALL;
+    Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions);
+    BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools());
+    TestedStandaloneTestStrategy standaloneTestStrategy =
+        new TestedStandaloneTestStrategy(executionOptions, binTools, tmpDirRoot);
+
+    // setup a test action
+    scratch.file("standalone/empty_test.sh", "this does not get executed, it is mocked out");
+    scratch.file(
+        "standalone/BUILD",
+        "sh_test(",
+        "    name = \"empty_test\",",
+        "    size = \"small\",",
+        "    srcs = [\"empty_test.sh\"],",
+        ")");
+    TestRunnerAction testRunnerAction = getTestAction("//standalone:empty_test");
+
+    SpawnResult expectedSpawnResult =
+        new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
+    when(spawnActionContext.beginExecution(any(), any()))
+        .then(
+            (invocation) -> {
+              ((ActionExecutionContext) invocation.getArgument(1)).getFileOutErr().printErr("Foo");
+              return SpawnContinuation.immediate(expectedSpawnResult);
+            });
+
+    FileOutErr outErr = createTempOutErr(tmpDirRoot);
+    ActionExecutionContext actionExecutionContext =
+        new FakeActionExecutionContext(outErr, spawnActionContext);
+
+    // actual StandaloneTestStrategy execution
+    execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy);
+
+    // check that the test stdout contains all the expected output
+    try {
+      String outData = FileSystemUtils.readContent(outErr.getOutputPath(), UTF_8);
+      assertThat(outData).contains("Foo");
+    } catch (IOException e) {
+      fail("Test stdout file missing: " + outErr.getOutputPath());
+    }
+  }
 }