From 0ff19c6d0adf3c0df94fff59ca3bd13cbcf99897 Mon Sep 17 00:00:00 2001
From: ulfjack <ulfjack@google.com>
Date: Tue, 21 May 2019 08:50:23 -0700
Subject: [PATCH] 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 #8320.

PiperOrigin-RevId: 249258656
---
 .../lib/exec/StandaloneTestStrategy.java      |  2 +-
 .../lib/exec/StandaloneTestStrategyTest.java  | 45 +++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

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 ccdb2068c1b2cb..01fbc1c9ae944d 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
@@ -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);
           }
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 11afe907165634..548626253de030 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
@@ -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());
+    }
+  }
 }