Add some tests to the LocalSpawnRunner, and make some small tweaks

- Only manually check the timeout if the process wrapper is not used
- Set the timeout correctly; the process API uses milliseconds
- flush the error output stream after writing
- return SIGALRM as exit code for timeout cases

PiperOrigin-RevId: 158374246

1 files changed, 71 insertions, 19 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
index 23d6515c0a..fb6fb9264c 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.exec.local;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -25,6 +26,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.io.ByteStreams;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputFileCache;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.actions.ResourceManager;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.Spawn;
@@ -39,6 +41,7 @@ import com.google.devtools.build.lib.shell.SubprocessBuilder;
 import com.google.devtools.build.lib.util.NetUtil;
 import com.google.devtools.build.lib.util.io.FileOutErr;
 import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import com.google.devtools.common.options.Options;
@@ -46,7 +49,11 @@ import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.SortedMap;
+import java.util.TreeMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Filter;
 import java.util.logging.LogRecord;
@@ -130,22 +137,9 @@ public class LocalSpawnRunnerTest {
     }
   }
 
-  private FileSystem fs;
-  private final ActionInputFileCache mockFileCache = mock(ActionInputFileCache.class);
-  private final ResourceManager resourceManager = ResourceManager.instanceForTestingOnly();
-
-  private Logger logger;
-  private AtomicInteger execCount = new AtomicInteger();
-  private FileOutErr outErr;
-  private long timeoutMillis = 0;
-  private boolean calledLockOutputFiles;
-
-  private final SpawnExecutionPolicy policy = new SpawnExecutionPolicy() {
-    @Override
-    public boolean shouldPrefetchInputsForLocalExecution(Spawn spawn) {
-      // TODO(ulfjack): Test local prefetching.
-      return false;
-    }
+  private final class SpawnExecutionPolicyForTesting implements SpawnExecutionPolicy {
+    private final List<ProgressStatus> reportedStatus = new ArrayList<>();
+    private final TreeMap<PathFragment, ActionInput> inputMapping = new TreeMap<>();
 
     @Override
     public void lockOutputFiles() throws InterruptedException {
@@ -169,14 +163,26 @@ public class LocalSpawnRunnerTest {
 
     @Override
     public SortedMap<PathFragment, ActionInput> getInputMapping() {
-      throw new UnsupportedOperationException();
+      return inputMapping;
     }
 
     @Override
     public void report(ProgressStatus state) {
-      // TODO(ulfjack): Test that the right calls are made.
+      reportedStatus.add(state);
     }
-  };
+  }
+
+  private FileSystem fs;
+  private final ActionInputFileCache mockFileCache = mock(ActionInputFileCache.class);
+  private final ResourceManager resourceManager = ResourceManager.instanceForTestingOnly();
+
+  private Logger logger;
+  private final AtomicInteger execCount = new AtomicInteger();
+  private FileOutErr outErr;
+  private long timeoutMillis = 0;
+  private boolean calledLockOutputFiles;
+
+  private final SpawnExecutionPolicyForTesting policy = new SpawnExecutionPolicyForTesting();
 
   @Before
   public final void setup() throws Exception  {
@@ -230,6 +236,8 @@ public class LocalSpawnRunnerTest {
     assertThat(captor.getValue().getTimeoutMillis()).isEqualTo(-1);
 
     assertThat(calledLockOutputFiles).isTrue();
+    assertThat(policy.reportedStatus)
+        .containsExactly(ProgressStatus.SCHEDULING, ProgressStatus.EXECUTING).inOrder();
   }
 
   @Test
@@ -305,6 +313,7 @@ public class LocalSpawnRunnerTest {
         logger, execCount, fs.getPath("/execroot"), ActionInputPrefetcher.NONE, options,
         resourceManager, USE_WRAPPER);
 
+    assertThat(fs.getPath("/out").createDirectory()).isTrue();
     outErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr"));
     SpawnResult result = runner.exec(SIMPLE_SPAWN, policy);
     verify(factory).create(any(SubprocessBuilder.class));
@@ -314,6 +323,9 @@ public class LocalSpawnRunnerTest {
     assertThat(result.getWallTimeMillis()).isEqualTo(0);
     assertThat(result.getExecutorHostName()).isEqualTo(NetUtil.findShortHostName());
 
+    assertThat(FileSystemUtils.readContent(fs.getPath("/out/stderr"), StandardCharsets.UTF_8))
+        .isEqualTo("Action failed to execute: java.io.IOException: I'm sorry, Dave\n");
+
     assertThat(calledLockOutputFiles).isTrue();
   }
 
@@ -374,4 +386,44 @@ public class LocalSpawnRunnerTest {
     }
     assertThat(calledLockOutputFiles).isTrue();
   }
+
+  @Test
+  public void checkPrefetchCalled() throws Exception {
+    Subprocess.Factory factory = mock(Subprocess.Factory.class);
+    when(factory.create(any())).thenReturn(new FinishedSubprocess(0));
+    SubprocessBuilder.setSubprocessFactory(factory);
+    ActionInputPrefetcher mockPrefetcher = mock(ActionInputPrefetcher.class);
+
+    LocalExecutionOptions options = Options.getDefaults(LocalExecutionOptions.class);
+    LocalSpawnRunner runner = new LocalSpawnRunner(
+        logger, execCount, fs.getPath("/execroot"), mockPrefetcher, options, resourceManager,
+        USE_WRAPPER);
+
+    timeoutMillis = 123 * 1000L;
+    outErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr"));
+    runner.exec(SIMPLE_SPAWN, policy);
+    verify(mockPrefetcher).prefetchFiles(any());
+  }
+
+  @Test
+  public void checkNoPrefetchCalled() throws Exception {
+    Subprocess.Factory factory = mock(Subprocess.Factory.class);
+    when(factory.create(any())).thenReturn(new FinishedSubprocess(0));
+    SubprocessBuilder.setSubprocessFactory(factory);
+    ActionInputPrefetcher mockPrefetcher = mock(ActionInputPrefetcher.class);
+    doThrow(new RuntimeException("Called prefetch!")).when(mockPrefetcher).prefetchFiles(any());
+
+    LocalExecutionOptions options = Options.getDefaults(LocalExecutionOptions.class);
+    LocalSpawnRunner runner = new LocalSpawnRunner(
+        logger, execCount, fs.getPath("/execroot"), mockPrefetcher, options, resourceManager,
+        USE_WRAPPER);
+
+    timeoutMillis = 123 * 1000L;
+    outErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr"));
+
+    Spawn spawn = new SpawnBuilder("/bin/echo", "Hi!")
+        .withExecutionInfo(ExecutionRequirements.DISABLE_LOCAL_PREFETCH, "").build();
+    // This would throw if the runner called prefetchFiles().
+    runner.exec(spawn, policy);
+  }
 }