From 7eadc73735d20dd27a20655d902e1456083d2649 Mon Sep 17 00:00:00 2001 From: ulfjack Date: Thu, 8 Jun 2017 05:56:29 -0400 Subject: 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 --- .../build/lib/exec/local/LocalSpawnRunnerTest.java | 90 +++++++++++++++++----- 1 file changed, 71 insertions(+), 19 deletions(-) (limited to 'src/test') 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 reportedStatus = new ArrayList<>(); + private final TreeMap inputMapping = new TreeMap<>(); @Override public void lockOutputFiles() throws InterruptedException { @@ -169,14 +163,26 @@ public class LocalSpawnRunnerTest { @Override public SortedMap 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); + } } -- cgit v1.2.3