From 0858ae1f6eb890c1e203a2aa21130ba34ca36a27 Mon Sep 17 00:00:00 2001 From: ulfjack Date: Fri, 27 Jul 2018 02:37:53 -0700 Subject: [PATCH] Add a flag to split test.xml generation into a separate Spawn At this time, this is only implemented for the StandaloneTestStrategy. This solves a race condition on Posix-like systems, where we cannot guarantee that the pipes are actually fully flushed to disk when the test process exits, and this can cause the test.xml to be empty, which makes it hard to debug issues. (The test.log files do not show up in normal CI systems, only the test.xml files.) Progress on #4608. PiperOrigin-RevId: 206292179 --- .../build/lib/analysis/BaseRuleClasses.java | 5 + .../skylark/SkylarkRuleClassFunctions.java | 6 + .../lib/analysis/test/TestActionBuilder.java | 4 + .../lib/analysis/test/TestRunnerAction.java | 7 + .../build/lib/exec/ExecutionOptions.java | 11 ++ .../lib/exec/StandaloneTestStrategy.java | 111 ++++++++++----- .../lib/analysis/mock/BazelAnalysisMock.java | 3 +- .../lib/exec/StandaloneTestStrategyTest.java | 126 ++++++++++++++++++ src/test/shell/shell_utils_symlinks_test.sh | 12 +- src/test/shell/shell_utils_test.sh | 12 +- src/test/shell/unittest_test.sh | 12 +- tools/test/BUILD | 6 + tools/test/generate-xml.sh | 55 ++++++++ tools/test/test-setup.sh | 21 ++- 14 files changed, 338 insertions(+), 53 deletions(-) create mode 100644 tools/test/generate-xml.sh diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java index 4c4c501d071893..2bd7daf1b9b62e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java @@ -174,6 +174,11 @@ public Object getDefault(AttributeMap rule) { .cfg(HostTransition.INSTANCE) .singleArtifact() .value(env.getToolsLabel("//tools/test:test_setup"))) + .add( + attr("$xml_generator_script", LABEL) + .cfg(HostTransition.INSTANCE) + .singleArtifact() + .value(env.getToolsLabel("//tools/test:test_xml_generator"))) .add( attr("$collect_coverage_script", LABEL) .cfg(HostTransition.INSTANCE) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java index 34cd54fd8ca940..fffbe7a5ea1b86 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java @@ -176,6 +176,12 @@ public static final RuleClass getTestBaseRule(String toolsRepository) { .cfg(HostTransition.INSTANCE) .singleArtifact() .value(labelCache.getUnchecked(toolsRepository + "//tools/test:test_setup"))) + .add( + attr("$xml_generator_script", LABEL) + .cfg(HostTransition.INSTANCE) + .singleArtifact() + .value( + labelCache.getUnchecked(toolsRepository + "//tools/test:test_xml_generator"))) .add( attr("$collect_coverage_script", LABEL) .cfg(HostTransition.INSTANCE) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java index 7fe18a23003e7a..40725da69d4a5a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java @@ -204,6 +204,9 @@ private TestParams createTestAction(int shards) { Artifact testSetupScript = ruleContext.getHostPrerequisiteArtifact("$test_setup_script"); inputsBuilder.add(testSetupScript); + Artifact testXmlGeneratorScript = + ruleContext.getHostPrerequisiteArtifact("$xml_generator_script"); + inputsBuilder.add(testXmlGeneratorScript); Artifact collectCoverageScript = null; TreeMap extraTestEnv = new TreeMap<>(); @@ -307,6 +310,7 @@ private TestParams createTestAction(int shards) { ruleContext.getActionOwner(), inputs, testSetupScript, + testXmlGeneratorScript, collectCoverageScript, testLog, cacheStatus, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java index 3548b9aafca770..5827488c09bb76 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java @@ -73,6 +73,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa private static final String GUID = "cc41f9d0-47a6-11e7-8726-eb6ce83a8cc8"; private final Artifact testSetupScript; + private final Artifact testXmlGeneratorScript; private final Artifact collectCoverageScript; private final BuildConfiguration configuration; private final TestConfiguration testConfiguration; @@ -135,6 +136,7 @@ private static ImmutableList list(Artifact... artifacts) { ActionOwner owner, Iterable inputs, Artifact testSetupScript, // Must be in inputs + Artifact testXmlGeneratorScript, // Must be in inputs @Nullable Artifact collectCoverageScript, // Must be in inputs, if not null Artifact testLog, Artifact cacheStatus, @@ -157,6 +159,7 @@ private static ImmutableList list(Artifact... artifacts) { configuration.getActionEnvironment()); Preconditions.checkState((collectCoverageScript == null) == (coverageArtifact == null)); this.testSetupScript = testSetupScript; + this.testXmlGeneratorScript = testXmlGeneratorScript; this.collectCoverageScript = collectCoverageScript; this.configuration = Preconditions.checkNotNull(configuration); this.testConfiguration = @@ -741,6 +744,10 @@ public Artifact getTestSetupScript() { return testSetupScript; } + public Artifact getTestXmlGeneratorScript() { + return testXmlGeneratorScript; + } + @Nullable public Artifact getCollectCoverageScript() { return collectCoverageScript; diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java index 953639932f01c4..0f9e2c27cbc382 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java @@ -309,6 +309,17 @@ public boolean usingLocalTestJobs() { ) public String executionLogFile; + @Option( + name = "experimental_split_xml_generation", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION}, + help = "If this flag is set, and a test action does not generate a test.xml file, then " + + "Bazel uses a separate action to generate a dummy test.xml file containing the test log. " + + "Otherwise, Bazel generates the test.xml as part of the test action." + ) + public boolean splitXmlGeneration; + /** Converter for the --flaky_test_attempts option. */ public static class TestAttemptsConverter extends PerLabelOptions.PerLabelOptionsConverter { private static final int MIN_VALUE = 1; 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 6c034d97b9eae5..cb3a4af6d95ee9 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 @@ -17,7 +17,9 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; @@ -30,15 +32,16 @@ import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.TestExecException; import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl; +import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.test.TestActionContext; 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.test.TestRunnerAction.ResolvedPaths; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; import com.google.devtools.build.lib.buildeventstream.TestFileNameConstants; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Reporter; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -50,6 +53,7 @@ import java.io.Closeable; import java.io.IOException; import java.time.Duration; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -99,10 +103,11 @@ public List exec( Path tmpDir = tmpDirRoot.getChild(TestStrategy.getTmpDirName(action)); Map env = setupEnvironment( action, actionExecutionContext.getClientEnv(), execRoot, runfilesDir, tmpDir); + if (executionOptions.splitXmlGeneration) { + env.put("EXPERIMENTAL_SPLIT_XML_GENERATION", "1"); + } Path workingDirectory = runfilesDir.getRelative(action.getRunfilesPrefix()); - ResolvedPaths resolvedPaths = action.resolve(execRoot); - Map executionInfo = new TreeMap<>(action.getTestProperties().getExecutionInfo()); if (!action.shouldCacheResult()) { @@ -336,7 +341,8 @@ protected StandaloneTestResult executeTest( long startTime = actionExecutionContext.getClock().currentTimeMillis(); SpawnActionContext spawnActionContext = actionExecutionContext.getContext(SpawnActionContext.class); - List spawnResults = ImmutableList.of(); + Path xmlOutputPath = action.resolve(actionExecutionContext.getExecRoot()).getXmlOutputPath(); + List spawnResults = new ArrayList<>(); BuildEventStreamProtos.TestResult.ExecutionInfo.Builder executionInfo = BuildEventStreamProtos.TestResult.ExecutionInfo.newBuilder(); try { @@ -347,28 +353,40 @@ protected StandaloneTestResult executeTest( Reporter.outErrForReporter(actionExecutionContext.getEventHandler()), testLogPath); } - spawnResults = spawnActionContext.exec(spawn, actionExecutionContext); - - builder - .setTestPassed(true) - .setStatus(BlazeTestStatus.PASSED) - .setPassedLog(testLogPath.getPathString()); - } catch (SpawnExecException e) { - // If this method returns normally, then the higher level will rerun the test (up to - // --flaky_test_attempts times). - if (e.isCatastrophic()) { - // Rethrow as the error was catastrophic and thus the build has to be halted. - throw e; + try { + spawnResults.addAll(spawnActionContext.exec(spawn, actionExecutionContext)); + builder + .setTestPassed(true) + .setStatus(BlazeTestStatus.PASSED) + .setPassedLog(testLogPath.getPathString()); + } catch (SpawnExecException e) { + // If this method returns normally, then the higher level will rerun the test (up to + // --flaky_test_attempts times). + if (e.isCatastrophic()) { + // Rethrow as the error was catastrophic and thus the build has to be halted. + throw e; + } + if (!e.getSpawnResult().setupSuccess()) { + // Rethrow as the test could not be run and thus there's no point in retrying. + throw e; + } + builder + .setTestPassed(false) + .setStatus(e.hasTimedOut() ? BlazeTestStatus.TIMEOUT : BlazeTestStatus.FAILED) + .addFailedLogs(testLogPath.getPathString()); + spawnResults.add(e.getSpawnResult()); } - if (!e.getSpawnResult().setupSuccess()) { - // Rethrow as the test could not be run and thus there's no point in retrying. - throw e; + // If the test did not create a test.xml, and --experimental_split_xml_generation is + // enabled, then we run a separate action to create a test.xml from test.log. + if (executionOptions.splitXmlGeneration + && action.getTestLog().getPath().exists() + && !xmlOutputPath.exists()) { + SpawnResult result = Iterables.getOnlyElement(spawnResults); + Spawn xmlGeneratingSpawn = createXmlGeneratingSpawn(action, result); + // We treat all failures to generate the test.xml here as catastrophic, and won't rerun + // the test if this fails. + spawnResults.addAll(spawnActionContext.exec(xmlGeneratingSpawn, actionExecutionContext)); } - builder - .setTestPassed(false) - .setStatus(e.hasTimedOut() ? BlazeTestStatus.TIMEOUT : BlazeTestStatus.FAILED) - .addFailedLogs(testLogPath.getPathString()); - spawnResults = ImmutableList.of(e.getSpawnResult()); } finally { long endTime = actionExecutionContext.getClock().currentTimeMillis(); long duration = endTime - startTime; @@ -376,7 +394,7 @@ protected StandaloneTestResult executeTest( if (!spawnResults.isEmpty()) { // The SpawnResult of a remotely cached or remotely executed action may not have walltime // set. We fall back to the time measured here for backwards compatibility. - SpawnResult primaryResult = Iterables.getOnlyElement(spawnResults); + SpawnResult primaryResult = spawnResults.iterator().next(); duration = primaryResult.getWallTime().orElse(Duration.ofMillis(duration)).toMillis(); extractExecutionInfo(primaryResult, builder, executionInfo); } @@ -390,11 +408,7 @@ protected StandaloneTestResult executeTest( } } - TestCase details = - parseTestResult( - action - .resolve(actionExecutionContext.getExecRoot()) - .getXmlOutputPath()); + TestCase details = parseTestResult(xmlOutputPath); if (details != null) { builder.setTestCase(details); } @@ -431,6 +445,43 @@ private static void extractExecutionInfo( } } + /** + * A spawn to generate a test.xml file from the test log. This is only used if the test does not + * generate a test.xml file itself. + */ + private Spawn createXmlGeneratingSpawn(TestRunnerAction action, SpawnResult result) { + List args = Lists.newArrayList(); + // TODO(ulfjack): This is incorrect for remote execution, where we need to consider the target + // configuration, not the machine Bazel happens to run on. Change this to something like: + // testAction.getConfiguration().getExecOS() == OS.WINDOWS + if (OS.getCurrent() == OS.WINDOWS) { + args.add(action.getShExecutable().getPathString()); + args.add("-c"); + args.add("$0 $*"); + } + args.add(action.getTestXmlGeneratorScript().getExecPath().getCallablePathString()); + args.add(action.getTestLog().getExecPathString()); + args.add(action.getXmlOutputPath().getPathString()); + args.add(Long.toString(result.getWallTime().orElse(Duration.ZERO).getSeconds())); + args.add(Integer.toString(result.exitCode())); + + return new SimpleSpawn( + action, + ImmutableList.copyOf(args), + ImmutableMap.of( + "PATH", "/usr/bin:/bin", + "TEST_SHARD_INDEX", Integer.toString(action.getShardNum()), + "TEST_TOTAL_SHARDS", Integer.toString(action.getExecutionSettings().getTotalShards()), + "TEST_NAME", action.getTestName()), + ImmutableMap.of(), + null, + ImmutableMap.of(), + /*inputs=*/ ImmutableList.of(action.getTestXmlGeneratorScript(), action.getTestLog()), + /*tools=*/ ImmutableList.of(), + /*outputs=*/ ImmutableList.of(ActionInputHelper.fromPath(action.getXmlOutputPath())), + SpawnAction.DEFAULT_RESOURCE_SET); + } + /** * Outputs test result to the stdout after test has finished (e.g. for --test_output=all or * --test_output=errors). Will also try to group output lines together (up to 10000 lines) so diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java index 0d399f2db357d0..123b58d3baebc7 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java @@ -126,8 +126,9 @@ public void setupMockClient(MockToolsConfig config) throws IOException { "/bazel_tools_workspace/tools/genrule/BUILD", "exports_files(['genrule-setup.sh'])"); config.create("/bazel_tools_workspace/tools/test/BUILD", - "filegroup(name = 'runtime', srcs = ['test-setup.sh'])", + "filegroup(name = 'runtime', srcs = ['test-setup.sh', 'test-xml-generator.sh'])", "filegroup(name = 'test_setup', srcs = ['test-setup.sh'])", + "filegroup(name = 'test_xml_generator', srcs = ['test-xml-generator.sh'])", "filegroup(name = 'collect_coverage', srcs = ['collect_coverage.sh'])", "filegroup(name='coverage_support', srcs=['collect_coverage.sh'])", "filegroup(name = 'coverage_report_generator', srcs = ['coverage_report_generator.sh'])"); 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 ab07910d6082ed..5351a956c3a0f6 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 @@ -19,6 +19,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.same; import static org.mockito.Mockito.when; @@ -30,6 +31,7 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; +import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; @@ -55,6 +57,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.invocation.InvocationOnMock; @@ -429,6 +432,7 @@ public void testThatTestLogAndOutputAreReturned() throws Exception { ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class); executionOptions.testOutput = TestOutputFormat.ERRORS; + executionOptions.splitXmlGeneration = false; Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions); BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools()); TestedStandaloneTestStrategy standaloneTestStrategy = @@ -533,6 +537,128 @@ public ActionExecutionContext answer(InvocationOnMock invocation) throws Throwab assertThat(errPath.exists()).isFalse(); } + @Test + public void testThatTestLogAndOutputAreReturnedWithSplitXmlGeneration() throws Exception { + + // setup a StandaloneTestStrategy + + ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class); + executionOptions.testOutput = TestOutputFormat.ERRORS; + executionOptions.splitXmlGeneration = true; + 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/failing_test.sh", "this does not get executed, it is mocked out"); + + scratch.file( + "standalone/BUILD", + "sh_test(", + " name = \"failing_test\",", + " size = \"small\",", + " srcs = [\"failing_test.sh\"],", + ")"); + + ConfiguredTarget configuredTarget = getConfiguredTarget("//standalone:failing_test"); + List 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())) + .thenAnswer( + new Answer() { + @SuppressWarnings("unchecked") + @Override + public ActionExecutionContext answer(InvocationOnMock invocation) throws Throwable { + FileOutErr outErr = (FileOutErr) invocation.getArguments()[0]; + try (OutputStream stream = outErr.getOutputStream()) { + stream.write("This will not appear in the test output: bla\n".getBytes(UTF_8)); + stream.write((TestLogHelper.HEADER_DELIMITER + "\n").getBytes(UTF_8)); + stream.write("This will appear in the test output: foo\n".getBytes(UTF_8)); + } + return actionExecutionContext; + } + }); + reporter.removeHandler(failFastHandler); + 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.getInputPath(any())).thenAnswer(this::getInputPathMock); + when(actionExecutionContext.getPathResolver()).thenReturn(ArtifactPathResolver.IDENTITY); + + Path outPath = tmpDirRoot.getRelative("test-out.txt"); + Path errPath = tmpDirRoot.getRelative("test-err.txt"); + FileOutErr outErr = new FileOutErr(outPath, errPath); + when(actionExecutionContext.getFileOutErr()).thenReturn(outErr); + + SpawnResult testSpawnResult = + new SpawnResult.Builder() + .setStatus(Status.NON_ZERO_EXIT) + .setExitCode(1) + .setRunnerName("test") + .build(); + when(spawnActionContext.exec(argThat(new ArgumentMatcher() { + @Override + public boolean matches(Object argument) { + return (argument instanceof Spawn) && ((Spawn) argument).getOutputFiles().size() != 1; + } + }), any())) + .thenThrow( + new SpawnExecException( + "Failure!!", + testSpawnResult, + /*forciblyRunRemotely=*/ false, + /*catastrophe=*/ false)); + + SpawnResult xmlGeneratorSpawnResult = + new SpawnResult.Builder() + .setStatus(Status.SUCCESS) + .setRunnerName("test") + .build(); + when(spawnActionContext.exec(argThat(new ArgumentMatcher() { + @Override + public boolean matches(Object argument) { + return (argument instanceof Spawn) && ((Spawn) argument).getOutputFiles().size() == 1; + } + }), any())) + .thenReturn(ImmutableList.of(xmlGeneratorSpawnResult)); + when(actionExecutionContext.getContext(same(SpawnActionContext.class))) + .thenReturn(spawnActionContext); + + // actual StandaloneTestStrategy execution + List spawnResults = + standaloneTestStrategy.exec(testRunnerAction, actionExecutionContext); + + // check that the rigged SpawnResult was returned + assertThat(spawnResults).containsExactly(testSpawnResult, xmlGeneratorSpawnResult); + // check that the test log contains all the output + String logData = FileSystemUtils.readContent(testRunnerAction.getTestLog().getPath(), UTF_8); + assertThat(logData).contains("bla"); + assertThat(logData).contains(TestLogHelper.HEADER_DELIMITER); + assertThat(logData).contains("foo"); + // check that the test stdout contains all the expected output + outErr.close(); // Create the output files. + String outData = FileSystemUtils.readContent(outPath, UTF_8); + assertThat(outData) + .contains("==================== Test output for //standalone:failing_test:"); + assertThat(outData).doesNotContain("bla"); + assertThat(outData).doesNotContain(TestLogHelper.HEADER_DELIMITER); + assertThat(outData).contains("foo"); + assertThat(outData) + .contains( + "================================================================================"); + assertThat(errPath.exists()).isFalse(); + } + @Test public void testEmptyOutputCreatesEmptyLogFile() throws Exception { // setup a StandaloneTestStrategy diff --git a/src/test/shell/shell_utils_symlinks_test.sh b/src/test/shell/shell_utils_symlinks_test.sh index dc822ac6200c4b..a673c67ad8ed8c 100755 --- a/src/test/shell/shell_utils_symlinks_test.sh +++ b/src/test/shell/shell_utils_symlinks_test.sh @@ -23,16 +23,16 @@ if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/ export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" elif [[ -f "$0.runfiles/MANIFEST" ]]; then export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" - elif [[ -f "$0.runfiles/io_bazel/tools/bash/runfiles/runfiles.bash" ]]; then + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + export RUNFILES_DIR="$0.runfiles" + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then export RUNFILES_DIR="$0.runfiles" - elif [[ -f "$TEST_SRCDIR/io_bazel/tools/bash/runfiles/runfiles.bash" ]]; then - export RUNFILES_DIR="$TEST_SRCDIR" fi fi -if [[ -f "${RUNFILES_DIR:-/dev/null}/io_bazel/tools/bash/runfiles/runfiles.bash" ]]; then - source "${RUNFILES_DIR}/io_bazel/tools/bash/runfiles/runfiles.bash" +if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then - source "$(grep -m1 "^io_bazel/tools/bash/runfiles/runfiles.bash " \ + source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" else echo >&2 "ERROR: cannot find //tools/bash/runfiles:runfiles.bash" diff --git a/src/test/shell/shell_utils_test.sh b/src/test/shell/shell_utils_test.sh index daf1e8a9f6ae07..4a008971cf5e89 100755 --- a/src/test/shell/shell_utils_test.sh +++ b/src/test/shell/shell_utils_test.sh @@ -23,16 +23,16 @@ if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/ export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" elif [[ -f "$0.runfiles/MANIFEST" ]]; then export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" - elif [[ -f "$0.runfiles/io_bazel/tools/bash/runfiles/runfiles.bash" ]]; then + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + export RUNFILES_DIR="$0.runfiles" + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then export RUNFILES_DIR="$0.runfiles" - elif [[ -f "$TEST_SRCDIR/io_bazel/tools/bash/runfiles/runfiles.bash" ]]; then - export RUNFILES_DIR="$TEST_SRCDIR" fi fi -if [[ -f "${RUNFILES_DIR:-/dev/null}/io_bazel/tools/bash/runfiles/runfiles.bash" ]]; then - source "${RUNFILES_DIR}/io_bazel/tools/bash/runfiles/runfiles.bash" +if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then - source "$(grep -m1 "^io_bazel/tools/bash/runfiles/runfiles.bash " \ + source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" else echo >&2 "ERROR: cannot find //tools/bash/runfiles:runfiles.bash" diff --git a/src/test/shell/unittest_test.sh b/src/test/shell/unittest_test.sh index ecca2417724ea0..b65b7aaacc7788 100755 --- a/src/test/shell/unittest_test.sh +++ b/src/test/shell/unittest_test.sh @@ -24,16 +24,16 @@ if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/ export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest" elif [[ -f "$0.runfiles/MANIFEST" ]]; then export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST" - elif [[ -f "$0.runfiles/io_bazel/tools/bash/runfiles/runfiles.bash" ]]; then + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + export RUNFILES_DIR="$0.runfiles" + elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then export RUNFILES_DIR="$0.runfiles" - elif [[ -f "$TEST_SRCDIR/io_bazel/tools/bash/runfiles/runfiles.bash" ]]; then - export RUNFILES_DIR="$TEST_SRCDIR" fi fi -if [[ -f "${RUNFILES_DIR:-/dev/null}/io_bazel/tools/bash/runfiles/runfiles.bash" ]]; then - source "${RUNFILES_DIR}/io_bazel/tools/bash/runfiles/runfiles.bash" +if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then + source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash" elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then - source "$(grep -m1 "^io_bazel/tools/bash/runfiles/runfiles.bash " \ + source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \ "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)" else echo >&2 "ERROR: cannot find //tools/bash/runfiles:runfiles.bash" diff --git a/tools/test/BUILD b/tools/test/BUILD index 4a4e56b2843e44..c6a83110330d89 100644 --- a/tools/test/BUILD +++ b/tools/test/BUILD @@ -2,6 +2,7 @@ package(default_visibility = ["//visibility:public"]) # Members of this filegroup shouldn't have duplicate basenames, otherwise # TestRunnerAction#getRuntimeArtifact() will get confused. +# Deprecated, do not use. filegroup( name = "runtime", srcs = ["test-setup.sh"], @@ -12,6 +13,11 @@ filegroup( srcs = ["test-setup.sh"], ) +filegroup( + name = "test_xml_generator", + srcs = ["generate-xml.sh"], +) + filegroup( name = "collect_coverage", srcs = ["collect_coverage.sh"], diff --git a/tools/test/generate-xml.sh b/tools/test/generate-xml.sh new file mode 100644 index 00000000000000..730b131f380f2e --- /dev/null +++ b/tools/test/generate-xml.sh @@ -0,0 +1,55 @@ +#!/bin/bash + +# Copyright 2018 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +TEST_LOG="$1" +XML_OUTPUT_FILE="$2" +DURATION_IN_SECONDS="$3" +EXIT_CODE="$4" + +# Keep this in sync with test-setup.sh! +function encode_output_file { + if [[ -f "$1" ]]; then + # Replace invalid XML characters and invalid sequence in CDATA + # cf. https://stackoverflow.com/a/7774512/4717701 + perl -CSDA -pe's/[^\x9\xA\xD\x20-\x{D7FF}\x{E000}-\x{FFFD}\x{10000}-\x{10FFFF}]+/?/g;' "$1" \ + | sed 's|]]>|]]>]]|g' + fi +} + +test_name="${TEST_BINARY#./}" +errors=0 +error_msg="" +if (( $EXIT_CODE != 0 )); then + errors=1 + error_msg="" +fi + +# Ensure that test shards have unique names in the xml output. +if [[ -n "${TEST_TOTAL_SHARDS+x}" ]] && ((TEST_TOTAL_SHARDS != 0)); then + ((shard_num=TEST_SHARD_INDEX+1)) + test_name="${test_name}"_shard_"${shard_num}"/"${TEST_TOTAL_SHARDS}" +fi + +cat <${XML_OUTPUT_FILE} + + + + ${error_msg} + + + +EOF + diff --git a/tools/test/test-setup.sh b/tools/test/test-setup.sh index d8f2a47b672b7d..d7bb931b5f91a3 100755 --- a/tools/test/test-setup.sh +++ b/tools/test/test-setup.sh @@ -152,6 +152,7 @@ if [[ -z "$no_echo" ]]; then echo "-----------------------------------------------------------------------------" fi +# Unused if EXPERIMENTAL_SPLIT_XML_GENERATION is set. function encode_output_file { if [ -f "$1" ]; then # Replace invalid XML characters and invalid sequence in CDATA @@ -161,6 +162,8 @@ function encode_output_file { fi } +# Unused if EXPERIMENTAL_SPLIT_XML_GENERATION is set. +# Keep this in sync with generate-xml.sh! function write_xml_output_file { local duration=$(expr $(date +%s) - $start) local errors=0 @@ -268,17 +271,27 @@ if [ "$has_tail" == true ] && [ -z "$no_echo" ]; then wait $pid exitCode=$? else - if [ -z "$COVERAGE_DIR" ]; then - "${TEST_PATH}" "$@" 2> >(tee -a "${XML_OUTPUT_FILE}.log" >&2) 1> >(tee -a "${XML_OUTPUT_FILE}.log") 2>&1 || exitCode=$? + if [[ "${EXPERIMENTAL_SPLIT_XML_GENERATION}" == "1" ]]; then + if [ -z "$COVERAGE_DIR" ]; then + "${TEST_PATH}" "$@" 2>&1 || exitCode=$? + else + "$1" "$TEST_PATH" "${@:3}" 2>&1 || exitCode=$? + fi else - "$1" "$TEST_PATH" "${@:3}" 2> >(tee -a "${XML_OUTPUT_FILE}.log" >&2) 1> >(tee -a "${XML_OUTPUT_FILE}.log") 2>&1 || exitCode=$? + if [ -z "$COVERAGE_DIR" ]; then + "${TEST_PATH}" "$@" 2> >(tee -a "${XML_OUTPUT_FILE}.log" >&2) 1> >(tee -a "${XML_OUTPUT_FILE}.log") 2>&1 || exitCode=$? + else + "$1" "$TEST_PATH" "${@:3}" 2> >(tee -a "${XML_OUTPUT_FILE}.log" >&2) 1> >(tee -a "${XML_OUTPUT_FILE}.log") 2>&1 || exitCode=$? + fi fi fi for signal in $signals; do trap - ${signal} done -write_xml_output_file +if [[ "${EXPERIMENTAL_SPLIT_XML_GENERATION}" != "1" ]]; then + write_xml_output_file +fi # Add all of the files from the undeclared outputs directory to the manifest. if [[ -n "$TEST_UNDECLARED_OUTPUTS_DIR" && -n "$TEST_UNDECLARED_OUTPUTS_MANIFEST" ]]; then