diff options
author | Benjamin Peterson <bp@benjamin.pe> | 2018-04-26 05:27:10 -0700 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-04-26 05:28:15 -0700 |
commit | 1bbeadc2e3b9dd09909c0a47f86cd878aede1392 (patch) | |
tree | 0cded323f8b0cf06fcef77279226354f6fbf8523 /src/test/java/com | |
parent | 425156edd865a554ef33af855dc8e4bd29160607 (diff) |
Add execution information to standalone test results.
In StandaloneTestStrategy, copy as much information as SpawnResult makes
available to us through to both the TestResultData and BEP's
TestResult.ExecutionInfo protos. One immediate consequence is that the UI and
BEP can tell you whether a test result was cached remotely.
I changed Executor.getEventHandler to return an ExtendedEventHandler because it
makes this change easier to test.
Closes #5081.
Change-Id: I94fefdcd2e029c81085076736ad13a4bdf1bae8f
PiperOrigin-RevId: 194383009
Diffstat (limited to 'src/test/java/com')
3 files changed, 282 insertions, 10 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java index 49a6aa0607..1266ab8803 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java @@ -167,7 +167,7 @@ public final class ActionsTestUtil { buildDriver, executor == null ? null : executor.getEventHandler())); } - public static ActionExecutionContext createContext(EventHandler eventHandler) { + public static ActionExecutionContext createContext(ExtendedEventHandler eventHandler) { DummyExecutor dummyExecutor = new DummyExecutor(eventHandler); return new ActionExecutionContext( dummyExecutor, diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java b/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java index 14076b606a..1d0cd9be99 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/DummyExecutor.java @@ -20,7 +20,7 @@ import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.clock.Clock; -import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.common.options.OptionsClassProvider; @@ -32,17 +32,17 @@ public final class DummyExecutor implements Executor { private final FileSystem fileSystem; private final Path inputDir; - private final EventHandler eventHandler; + private final ExtendedEventHandler eventHandler; public DummyExecutor(FileSystem fileSystem, Path inputDir) { this(fileSystem, inputDir, null); } - public DummyExecutor(EventHandler eventHandler) { + public DummyExecutor(ExtendedEventHandler eventHandler) { this(null, null, eventHandler); } - public DummyExecutor(FileSystem fileSystem, Path inputDir, EventHandler eventHandler) { + public DummyExecutor(FileSystem fileSystem, Path inputDir, ExtendedEventHandler eventHandler) { this.fileSystem = fileSystem; this.inputDir = inputDir; this.eventHandler = eventHandler; @@ -74,7 +74,7 @@ public final class DummyExecutor implements Executor { } @Override - public EventHandler getEventHandler() { + public ExtendedEventHandler getEventHandler() { return eventHandler; } 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 34058a42f5..0f0a02363d 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 @@ -24,6 +24,7 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.common.collect.MoreCollectors; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; @@ -36,10 +37,12 @@ import com.google.devtools.build.lib.analysis.test.TestResult; import com.google.devtools.build.lib.analysis.test.TestRunnerAction; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.clock.BlazeClock; +import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.exec.TestStrategy.TestOutputFormat; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; import com.google.devtools.common.options.Options; import java.io.IOException; import java.io.OutputStream; @@ -59,6 +62,8 @@ import org.mockito.stubbing.Answer; public final class StandaloneTestStrategyTest extends BuildViewTestCase { private static class TestedStandaloneTestStrategy extends StandaloneTestStrategy { + TestResult postedResult = null; + public TestedStandaloneTestStrategy( ExecutionOptions executionOptions, BinTools binTools, Path tmpDirRoot) { super(executionOptions, binTools, tmpDirRoot); @@ -67,7 +72,7 @@ public final class StandaloneTestStrategyTest extends BuildViewTestCase { @Override protected void postTestResult(ActionExecutionContext actionExecutionContext, TestResult result) throws IOException { - // Make postTestResult a no-op for testing purposes + postedResult = result; } } @@ -75,13 +80,15 @@ public final class StandaloneTestStrategyTest extends BuildViewTestCase { @Mock private SpawnActionContext spawnActionContext; + private StoredEventHandler storedEvents = new StoredEventHandler(); + @Before public final void setUp() throws Exception { MockitoAnnotations.initMocks(this); } @Test - public void testSpawnResultsAreReturned() throws Exception { + public void testRunTestOnce() throws Exception { // setup a StandaloneTestStrategy @@ -117,8 +124,7 @@ public final class StandaloneTestStrategyTest extends BuildViewTestCase { when(actionExecutionContext.withFileOutErr(any())).thenReturn(actionExecutionContext); when(actionExecutionContext.getExecRoot()).thenReturn(outputBase.getRelative("execroot")); when(actionExecutionContext.getClientEnv()).thenReturn(ImmutableMap.of()); - when(actionExecutionContext.getEventHandler()).thenReturn(reporter); - when(actionExecutionContext.getEventBus()).thenReturn(eventBus); + when(actionExecutionContext.getEventHandler()).thenReturn(storedEvents); when(actionExecutionContext.getInputPath(any())).thenAnswer(this::getInputPathMock); SpawnResult expectedSpawnResult = @@ -135,8 +141,274 @@ public final class StandaloneTestStrategyTest extends BuildViewTestCase { List<SpawnResult> spawnResults = standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext); + assertThat(spawnResults).containsExactly(expectedSpawnResult); + TestResult result = standaloneTestStrategy.postedResult; + assertThat(result).isNotNull(); + assertThat(result.isCached()).isFalse(); + assertThat(result.getTestStatusArtifact()).isEqualTo(testStatusArtifact); + assertThat(result.getData().getTestPassed()).isTrue(); + assertThat(result.getData().getRemotelyCached()).isFalse(); + assertThat(result.getData().getIsRemoteStrategy()).isFalse(); + assertThat(result.getData().getRunDurationMillis()).isEqualTo(10); + assertThat(result.getData().getTestTimesList()).containsExactly(10L); + TestAttempt attempt = + storedEvents + .getPosts() + .stream() + .filter(TestAttempt.class::isInstance) + .map(TestAttempt.class::cast) + .collect(MoreCollectors.onlyElement()); + assertThat(attempt.getExecutionInfo().getStrategy()).isEqualTo("test"); + assertThat(attempt.getExecutionInfo().getHostname()).isEqualTo(""); + } + + @Test + public void testRunFlakyTest() throws Exception { + + // setup a StandaloneTestStrategy + + ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS; + 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/simple_test.sh", "this does not get executed, it is mocked out"); + + scratch.file( + "standalone/BUILD", + "sh_test(", + " name = \"simple_test\",", + " size = \"small\",", + " srcs = [\"simple_test.sh\"],", + " flaky = True,", + ")"); + + ConfiguredTarget configuredTarget = getConfiguredTarget("//standalone:simple_test"); + List<Artifact> testStatusArtifacts = + configuredTarget.getProvider(TestProvider.class).getTestParams().getTestStatusArtifacts(); + Artifact testStatusArtifact = Iterables.getOnlyElement(testStatusArtifacts); + TestRunnerAction testRunnerAction = (TestRunnerAction) getGeneratingAction(testStatusArtifact); + assertThat(testRunnerAction.getTestProperties().isFlaky()).isTrue(); + FileSystemUtils.createDirectoryAndParents( + testRunnerAction.getTestLog().getPath().getParentDirectory()); + + // setup a mock ActionExecutionContext + + when(actionExecutionContext.getClock()).thenReturn(BlazeClock.instance()); + when(actionExecutionContext.withFileOutErr(any())).thenReturn(actionExecutionContext); + when(actionExecutionContext.getExecRoot()).thenReturn(outputBase.getRelative("execroot")); + when(actionExecutionContext.getClientEnv()).thenReturn(ImmutableMap.of()); + when(actionExecutionContext.getEventHandler()).thenReturn(storedEvents); + when(actionExecutionContext.getInputPath(any())).thenAnswer(this::getInputPathMock); + + SpawnResult failSpawnResult = + new SpawnResult.Builder() + .setStatus(Status.NON_ZERO_EXIT) + .setExitCode(1) + .setWallTime(Duration.ofMillis(10)) + .setRunnerName("test") + .build(); + SpawnResult passSpawnResult = + new SpawnResult.Builder() + .setStatus(Status.SUCCESS) + .setWallTime(Duration.ofMillis(15)) + .setRunnerName("test") + .build(); + when(spawnActionContext.exec(any(), any())) + .thenThrow(new SpawnExecException("test failed", failSpawnResult, false)) + .thenReturn(ImmutableList.of(passSpawnResult)); + + when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext); + + // actual StandaloneTestStrategy execution + List<SpawnResult> spawnResults = + standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext); + + assertThat(spawnResults).containsExactly(passSpawnResult); + TestResult result = standaloneTestStrategy.postedResult; + assertThat(result).isNotNull(); + assertThat(result.isCached()).isFalse(); + assertThat(result.getTestStatusArtifact()).isEqualTo(testStatusArtifact); + assertThat(result.getData().getStatus()).isEqualTo(BlazeTestStatus.FLAKY); + assertThat(result.getData().getTestPassed()).isTrue(); + assertThat(result.getData().getRemotelyCached()).isFalse(); + assertThat(result.getData().getIsRemoteStrategy()).isFalse(); + assertThat(result.getData().getRunDurationMillis()).isEqualTo(15L); + assertThat(result.getData().getTestTimesList()).containsExactly(10L, 15L); + List<TestAttempt> attempts = + storedEvents + .getPosts() + .stream() + .filter(TestAttempt.class::isInstance) + .map(TestAttempt.class::cast) + .collect(ImmutableList.toImmutableList()); + assertThat(attempts).hasSize(2); + TestAttempt failedAttempt = attempts.get(0); + assertThat(failedAttempt.getExecutionInfo().getStrategy()).isEqualTo("test"); + assertThat(failedAttempt.getExecutionInfo().getHostname()).isEqualTo(""); + assertThat(failedAttempt.getStatus()).isEqualTo(BlazeTestStatus.FAILED); + assertThat(failedAttempt.getExecutionInfo().getCachedRemotely()).isFalse(); + TestAttempt okAttempt = attempts.get(1); + assertThat(okAttempt.getStatus()).isEqualTo(BlazeTestStatus.PASSED); + assertThat(okAttempt.getExecutionInfo().getStrategy()).isEqualTo("test"); + assertThat(okAttempt.getExecutionInfo().getHostname()).isEqualTo(""); + } + + @Test + public void testRunTestRemotely() throws Exception { + + // setup a StandaloneTestStrategy + + ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS; + 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/simple_test.sh", "this does not get executed, it is mocked out"); + + scratch.file( + "standalone/BUILD", + "sh_test(", + " name = \"simple_test\",", + " size = \"small\",", + " srcs = [\"simple_test.sh\"],", + ")"); + + ConfiguredTarget configuredTarget = getConfiguredTarget("//standalone:simple_test"); + List<Artifact> testStatusArtifacts = + configuredTarget.getProvider(TestProvider.class).getTestParams().getTestStatusArtifacts(); + Artifact testStatusArtifact = Iterables.getOnlyElement(testStatusArtifacts); + TestRunnerAction testRunnerAction = (TestRunnerAction) getGeneratingAction(testStatusArtifact); + FileSystemUtils.createDirectoryAndParents( + testRunnerAction.getTestLog().getPath().getParentDirectory()); + + // setup a mock ActionExecutionContext + + when(actionExecutionContext.getClock()).thenReturn(BlazeClock.instance()); + when(actionExecutionContext.withFileOutErr(any())).thenReturn(actionExecutionContext); + when(actionExecutionContext.getExecRoot()).thenReturn(outputBase.getRelative("execroot")); + when(actionExecutionContext.getClientEnv()).thenReturn(ImmutableMap.of()); + when(actionExecutionContext.getEventHandler()).thenReturn(storedEvents); + when(actionExecutionContext.getInputPath(any())).thenAnswer(this::getInputPathMock); + + SpawnResult expectedSpawnResult = + new SpawnResult.Builder() + .setStatus(Status.SUCCESS) + .setWallTime(Duration.ofMillis(10)) + .setRunnerName("remote") + .setExecutorHostname("a-remote-host") + .build(); + when(spawnActionContext.exec(any(), any())).thenReturn(ImmutableList.of(expectedSpawnResult)); + + when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext); + + // actual StandaloneTestStrategy execution + List<SpawnResult> spawnResults = + standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext); + + assertThat(spawnResults).containsExactly(expectedSpawnResult); + TestResult result = standaloneTestStrategy.postedResult; + assertThat(result).isNotNull(); + assertThat(result.isCached()).isFalse(); + assertThat(result.getTestStatusArtifact()).isEqualTo(testStatusArtifact); + assertThat(result.getData().getTestPassed()).isTrue(); + assertThat(result.getData().getRemotelyCached()).isFalse(); + assertThat(result.getData().getIsRemoteStrategy()).isTrue(); + assertThat(result.getData().getRunDurationMillis()).isEqualTo(10); + assertThat(result.getData().getTestTimesList()).containsExactly(10L); + TestAttempt attempt = + storedEvents + .getPosts() + .stream() + .filter(TestAttempt.class::isInstance) + .map(TestAttempt.class::cast) + .collect(MoreCollectors.onlyElement()); + assertThat(attempt.getStatus()).isEqualTo(BlazeTestStatus.PASSED); + assertThat(attempt.getExecutionInfo().getStrategy()).isEqualTo("remote"); + assertThat(attempt.getExecutionInfo().getHostname()).isEqualTo("a-remote-host"); + } + + @Test + public void testRunRemotelyCachedTest() throws Exception { + + // setup a StandaloneTestStrategy + + ExecutionOptions executionOptions = ExecutionOptions.DEFAULTS; + 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/simple_test.sh", "this does not get executed, it is mocked out"); + + scratch.file( + "standalone/BUILD", + "sh_test(", + " name = \"simple_test\",", + " size = \"small\",", + " srcs = [\"simple_test.sh\"],", + ")"); + + ConfiguredTarget configuredTarget = getConfiguredTarget("//standalone:simple_test"); + List<Artifact> testStatusArtifacts = + configuredTarget.getProvider(TestProvider.class).getTestParams().getTestStatusArtifacts(); + Artifact testStatusArtifact = Iterables.getOnlyElement(testStatusArtifacts); + TestRunnerAction testRunnerAction = (TestRunnerAction) getGeneratingAction(testStatusArtifact); + FileSystemUtils.createDirectoryAndParents( + testRunnerAction.getTestLog().getPath().getParentDirectory()); + + // setup a mock ActionExecutionContext + + when(actionExecutionContext.getClock()).thenReturn(BlazeClock.instance()); + when(actionExecutionContext.withFileOutErr(any())).thenReturn(actionExecutionContext); + when(actionExecutionContext.getExecRoot()).thenReturn(outputBase.getRelative("execroot")); + when(actionExecutionContext.getClientEnv()).thenReturn(ImmutableMap.of()); + when(actionExecutionContext.getEventHandler()).thenReturn(storedEvents); + when(actionExecutionContext.getInputPath(any())).thenAnswer(this::getInputPathMock); + + SpawnResult expectedSpawnResult = + new SpawnResult.Builder() + .setStatus(Status.SUCCESS) + .setCacheHit(true) + .setWallTime(Duration.ofMillis(10)) + .setRunnerName("remote cache") + .build(); + when(spawnActionContext.exec(any(), any())).thenReturn(ImmutableList.of(expectedSpawnResult)); + + when(actionExecutionContext.getSpawnActionContext(any())).thenReturn(spawnActionContext); + + // actual StandaloneTestStrategy execution + List<SpawnResult> spawnResults = + standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext); + // check that the rigged SpawnResult was returned assertThat(spawnResults).containsExactly(expectedSpawnResult); + TestResult result = standaloneTestStrategy.postedResult; + assertThat(result).isNotNull(); + assertThat(result.isCached()).isFalse(); + assertThat(result.getTestStatusArtifact()).isEqualTo(testStatusArtifact); + assertThat(result.getData().getTestPassed()).isTrue(); + assertThat(result.getData().getRemotelyCached()).isTrue(); + assertThat(result.getData().getIsRemoteStrategy()).isFalse(); + assertThat(result.getData().getRunDurationMillis()).isEqualTo(10); + assertThat(result.getData().getTestTimesList()).containsExactly(10L); + TestAttempt attempt = + storedEvents + .getPosts() + .stream() + .filter(TestAttempt.class::isInstance) + .map(TestAttempt.class::cast) + .collect(MoreCollectors.onlyElement()); + assertThat(attempt.getExecutionInfo().getStrategy()).isEqualTo("remote cache"); + assertThat(attempt.getExecutionInfo().getHostname()).isEqualTo(""); } @Test |