diff options
14 files changed, 338 insertions, 53 deletions
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 4c4c501d07..2bd7daf1b9 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 @@ -175,6 +175,11 @@ public class BaseRuleClasses { .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) .singleArtifact() 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 34cd54fd8c..fffbe7a5ea 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 @@ -177,6 +177,12 @@ public class SkylarkRuleClassFunctions implements SkylarkRuleFunctionsApi<Artifa .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) .singleArtifact() 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 7fe18a2300..40725da69d 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 @@ public final class TestActionBuilder { Artifact testSetupScript = ruleContext.getHostPrerequisiteArtifact("$test_setup_script"); inputsBuilder.add(testSetupScript); + Artifact testXmlGeneratorScript = + ruleContext.getHostPrerequisiteArtifact("$xml_generator_script"); + inputsBuilder.add(testXmlGeneratorScript); Artifact collectCoverageScript = null; TreeMap<String, String> extraTestEnv = new TreeMap<>(); @@ -307,6 +310,7 @@ public final class TestActionBuilder { 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 3548b9aafc..5827488c09 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 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa ActionOwner owner, Iterable<Artifact> 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 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa 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 class TestRunnerAction extends AbstractAction implements NotifyOnActionCa 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 953639932f..0f9e2c27cb 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 class ExecutionOptions extends OptionsBase { ) 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 6c034d97b9..cb3a4af6d9 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 @@ package com.google.devtools.build.lib.exec; 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.SpawnActionContext; 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 com.google.devtools.build.lib.view.test.TestStatus.TestResultData; 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 class StandaloneTestStrategy extends TestStrategy { Path tmpDir = tmpDirRoot.getChild(TestStrategy.getTmpDirName(action)); Map<String, String> 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<String, String> executionInfo = new TreeMap<>(action.getTestProperties().getExecutionInfo()); if (!action.shouldCacheResult()) { @@ -336,7 +341,8 @@ public class StandaloneTestStrategy extends TestStrategy { long startTime = actionExecutionContext.getClock().currentTimeMillis(); SpawnActionContext spawnActionContext = actionExecutionContext.getContext(SpawnActionContext.class); - List<SpawnResult> spawnResults = ImmutableList.of(); + Path xmlOutputPath = action.resolve(actionExecutionContext.getExecRoot()).getXmlOutputPath(); + List<SpawnResult> spawnResults = new ArrayList<>(); BuildEventStreamProtos.TestResult.ExecutionInfo.Builder executionInfo = BuildEventStreamProtos.TestResult.ExecutionInfo.newBuilder(); try { @@ -347,28 +353,40 @@ public class StandaloneTestStrategy extends TestStrategy { 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 @@ public class StandaloneTestStrategy extends TestStrategy { 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 @@ public class StandaloneTestStrategy extends TestStrategy { } } - TestCase details = - parseTestResult( - action - .resolve(actionExecutionContext.getExecRoot()) - .getXmlOutputPath()); + TestCase details = parseTestResult(xmlOutputPath); if (details != null) { builder.setTestCase(details); } @@ -432,6 +446,43 @@ public class StandaloneTestStrategy extends TestStrategy { } /** + * 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<String> 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.<Artifact>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 * parallel test outputs will not get interleaved. 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 0d399f2db3..123b58d3ba 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 final class BazelAnalysisMock extends AnalysisMock { "/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 ab07910d60..5351a956c3 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 com.google.devtools.build.lib.testutil.TestConstants.WORKSPACE_NAM 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.ActionExecutionContext; 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.Before; 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 final class StandaloneTestStrategyTest extends BuildViewTestCase { 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 = @@ -534,6 +538,128 @@ public final class StandaloneTestStrategyTest extends BuildViewTestCase { } @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<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())) + .thenAnswer( + new Answer<ActionExecutionContext>() { + @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<Spawn>() { + @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<Spawn>() { + @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<SpawnResult> 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 ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class); diff --git a/src/test/shell/shell_utils_symlinks_test.sh b/src/test/shell/shell_utils_symlinks_test.sh index dc822ac620..a673c67ad8 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 daf1e8a9f6..4a008971cf 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 ecca241772..b65b7aaacc 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 4a4e56b284..c6a8311033 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"], @@ -13,6 +14,11 @@ filegroup( ) 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 0000000000..730b131f38 --- /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|]]>|]]>]]<![CDATA[>|g' + fi +} + +test_name="${TEST_BINARY#./}" +errors=0 +error_msg="" +if (( $EXIT_CODE != 0 )); then + errors=1 + error_msg="<error message=\"exited with error code $EXIT_CODE\"></error>" +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 <<EOF >${XML_OUTPUT_FILE} +<?xml version="1.0" encoding="UTF-8"?> +<testsuites> +<testsuite name="${test_name}" tests="1" failures="0" errors="${errors}"> + <testcase name="${test_name}" status="run" duration="${DURATION_IN_SECONDS}" time="${DURATION_IN_SECONDS}">${error_msg}</testcase> + <system-out><![CDATA[$(encode_output_file "${TEST_LOG}")]]></system-out> +</testsuite> +</testsuites> +EOF + diff --git a/tools/test/test-setup.sh b/tools/test/test-setup.sh index d8f2a47b67..d7bb931b5f 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 |