diff --git a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java index 9e91b27f134f8f..490d61935daddb 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/SimpleSpawn.java @@ -32,6 +32,7 @@ @Immutable public final class SimpleSpawn implements Spawn { private final ActionExecutionMetadata owner; + private final String mnemonic; private final ImmutableList arguments; private final ImmutableMap environment; private final ImmutableMap executionInfo; @@ -58,7 +59,38 @@ private SimpleSpawn( @Nullable ResourceSet localResources, @Nullable LocalResourcesSupplier localResourcesSupplier, PathMapper pathMapper) { + this( + owner, + arguments, + environment, + executionInfo, + filesetMappings, + inputs, + tools, + outputs, + mandatoryOutputs, + localResources, + localResourcesSupplier, + pathMapper, + owner.getMnemonic()); + } + + private SimpleSpawn( + ActionExecutionMetadata owner, + ImmutableList arguments, + ImmutableMap environment, + ImmutableMap executionInfo, + ImmutableMap filesetMappings, + NestedSet inputs, + NestedSet tools, + Collection outputs, + @Nullable final Set mandatoryOutputs, + @Nullable ResourceSet localResources, + @Nullable LocalResourcesSupplier localResourcesSupplier, + PathMapper pathMapper, + String mnemonic) { this.owner = Preconditions.checkNotNull(owner); + this.mnemonic = Preconditions.checkNotNull(mnemonic); this.arguments = Preconditions.checkNotNull(arguments); this.environment = Preconditions.checkNotNull(environment); this.executionInfo = Preconditions.checkNotNull(executionInfo); @@ -82,6 +114,36 @@ private SimpleSpawn( this.pathMapper = pathMapper; } + /** Creates a spawn whose mnemonic differs from its resource owner's mnemonic. */ + @SuppressWarnings("TooManyParameters") + public static SimpleSpawn withMnemonic( + ActionExecutionMetadata owner, + ImmutableList arguments, + ImmutableMap environment, + ImmutableMap executionInfo, + ImmutableMap filesetMappings, + NestedSet inputs, + NestedSet tools, + Collection outputs, + @Nullable Set mandatoryOutputs, + ResourceSet localResources, + String mnemonic) { + return new SimpleSpawn( + owner, + arguments, + environment, + executionInfo, + filesetMappings, + inputs, + tools, + outputs, + mandatoryOutputs, + localResources, + /* localResourcesSupplier= */ null, + PathMapper.NOOP, + mnemonic); + } + public SimpleSpawn( ActionExecutionMetadata owner, ImmutableList arguments, @@ -292,7 +354,7 @@ public PathMapper getPathMapper() { @Override public String getMnemonic() { - return owner.getMnemonic(); + return mnemonic; } @Override 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 7069c4b54b1da7..bf9f44c7579717 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 @@ -258,6 +258,9 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) .cfg(ExecutionTransitionFactory.createFactory()) .singleArtifact() .value(env.getToolsLabel("//tools/test:xml_writer"))) + .add( + attr("$ensure_xml", LABEL) + .value(env.getToolsLabel("//tools/test:ensure_xml"))) .add( attr("$test_runtime", LABEL_LIST) .cfg(ExecutionTransitionFactory.createFactory()) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index f52d7f16d0f684..316e3ab9ad25e5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java @@ -263,6 +263,9 @@ public static RuleClass getTestBaseRule(RuleDefinitionEnvironment env) { .cfg(ExecutionTransitionFactory.createFactory()) .singleArtifact() .value(labelCache.get(toolsRepository + "//tools/test:xml_writer"))) + .add( + attr("$ensure_xml", LABEL) + .value(labelCache.get(toolsRepository + "//tools/test:ensure_xml"))) .add( attr("$test_runtime", LABEL_LIST) .cfg(ExecutionTransitionFactory.createFactory()) 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 2d2ae14f71eac4..c027d232af66a7 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 @@ -39,14 +39,19 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.actions.LazyWriteNestedSetOfTupleAction; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; +import com.google.devtools.build.lib.analysis.platform.ConstraintSettingInfo; +import com.google.devtools.build.lib.analysis.platform.PlatformInfo; import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions.CancelConcurrentTests; import com.google.devtools.build.lib.analysis.test.TestProvider.TestParams; import com.google.devtools.build.lib.analysis.test.TestProvider.TestParams.CoverageParams; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.TestTimeout; import com.google.devtools.build.lib.packages.Type; +import com.google.devtools.build.lib.util.CPU; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.List; @@ -75,6 +80,10 @@ public final class TestActionBuilder { // reported source file is the same as the actual source path it will not be included in the file. private static final String COVERAGE_REPORTED_TO_ACTUAL_SOURCES_FILE = "COVERAGE_REPORTED_TO_ACTUAL_SOURCES_FILE"; + private static final ConstraintSettingInfo OS_CONSTRAINT_SETTING = + ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("@platforms//os:os")); + private static final ConstraintSettingInfo CPU_CONSTRAINT_SETTING = + ConstraintSettingInfo.create(Label.parseCanonicalUnchecked("@platforms//cpu:cpu")); private final RuleContext ruleContext; private final ImmutableList.Builder additionalTools; @@ -176,6 +185,53 @@ private ActionOwner getTestActionOwner(boolean useTargetPlatformForTests) { : ruleContext.getActionOwner(); } + @Nullable + private Artifact getEnsureXmlExecutable(ActionOwner actionOwner) { + String platformName = getEnsureXmlPlatformName(actionOwner.getExecutionPlatform()); + if (platformName == null) { + return null; + } + String filename = "ensure_xml_" + platformName; + return ruleContext.getPrerequisiteArtifacts("$ensure_xml").list().stream() + .filter(artifact -> artifact.getFilename().equals(filename)) + .findFirst() + .orElse(null); + } + + @Nullable + private static String getEnsureXmlPlatformName(@Nullable PlatformInfo executionPlatform) { + if (executionPlatform == null) { + return getEnsureXmlPlatformName( + OS.getCurrent().getCanonicalName(), CPU.getCurrent().getCanonicalName()); + } + + if (!executionPlatform.constraints().has(OS_CONSTRAINT_SETTING) + || !executionPlatform.constraints().has(CPU_CONSTRAINT_SETTING)) { + return null; + } + return getEnsureXmlPlatformName( + executionPlatform.constraints().get(OS_CONSTRAINT_SETTING).label().getName(), + executionPlatform.constraints().get(CPU_CONSTRAINT_SETTING).label().getName()); + } + + @Nullable + private static String getEnsureXmlPlatformName(String os, String cpu) { + return switch (os) { + case "linux" -> + switch (cpu) { + case "x86_64", "amd64" -> "linux_amd64"; + case "aarch64", "arm64" -> "linux_arm64"; + default -> null; + }; + case "osx", "darwin" -> + switch (cpu) { + case "aarch64", "arm64" -> "darwin_arm64"; + default -> null; + }; + default -> null; + }; + } + public static int getShardCount(RuleContext ruleContext) { int explicitShardCount = ruleContext.attributes().get("shard_count", Type.INTEGER).toIntUnchecked(); @@ -216,6 +272,7 @@ private TestParams createTestAction() TestConfiguration testConfiguration = config.getFragment(TestConfiguration.class); AnalysisEnvironment env = ruleContext.getAnalysisEnvironment(); ArtifactRoot root = ruleContext.getTestLogsDirectory(); + ActionOwner actionOwner = getTestActionOwner(testConfiguration.useTargetPlatformForTests()); final boolean isUsingTestWrapperInsteadOfTestSetupScript = ruleContext.isExecutedOnWindows(); @@ -246,6 +303,14 @@ private TestParams createTestAction() ? ruleContext.getPrerequisiteArtifact("$xml_writer") : ruleContext.getPrerequisiteArtifact("$xml_generator_script"); inputsBuilder.add(testXmlGeneratorExecutable); + Artifact ensureXmlExecutable = null; + if (testConfiguration.getTestXmlMissingBehavior() + == TestConfiguration.TestXmlMissingBehavior.WORKAROUND) { + ensureXmlExecutable = getEnsureXmlExecutable(actionOwner); + if (ensureXmlExecutable != null) { + inputsBuilder.add(ensureXmlExecutable); + } + } FilesToRunProvider collectCoverageScript = null; TreeMap coverageTestEnv = new TreeMap<>(); @@ -356,8 +421,6 @@ private TestParams createTestAction() ImmutableList.Builder coverageArtifacts = ImmutableList.builder(); ImmutableList.Builder testOutputs = ImmutableList.builder(); - ActionOwner actionOwner = getTestActionOwner(testConfiguration.useTargetPlatformForTests()); - // Use 1-based indices for user friendliness. for (int shard = 0; shard < shardRuns; shard++) { String shardDir = @@ -416,6 +479,7 @@ private TestParams createTestAction() runfilesSupport.getRunfilesMiddleman(), testActionExecutable, testXmlGeneratorExecutable, + ensureXmlExecutable, collectCoverageScript, testLog, testXml, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java index aaccb729e807c8..b7c637198e6e34 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.RegexFilter; import com.google.devtools.common.options.BoolOrEnumConverter; +import com.google.devtools.common.options.EnumConverter; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDefinition; import com.google.devtools.common.options.OptionDocumentationCategory; @@ -53,6 +54,20 @@ /** Test-related options. */ @RequiresOptions(options = {TestConfiguration.TestOptions.class}) public class TestConfiguration extends Fragment { + public enum TestXmlMissingBehavior { + LEGACY, + FAIL, + WARN, + WORKAROUND; + + /** Converts to {@link TestXmlMissingBehavior}. */ + public static class Converter extends EnumConverter { + public Converter() { + super(TestXmlMissingBehavior.class, "test XML missing behavior"); + } + } + } + public static final OptionsDiffPredicate SHOULD_INVALIDATE_FOR_OPTION_DIFF = (options, changedOption, oldValue, newValue) -> { if (TestOptions.ALWAYS_INVALIDATE_WHEN_CHANGED.contains(changedOption)) { @@ -141,6 +156,21 @@ public static class TestOptions extends FragmentOptions { + " upon first failure.") public boolean testRunnerFailFast; + @Option( + name = "experimental_test_xml_missing_behavior", + defaultValue = "legacy", + converter = TestXmlMissingBehavior.Converter.class, + documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY, + effectTags = {OptionEffectTag.EXECUTION, OptionEffectTag.AFFECTS_OUTPUTS}, + metadataTags = {OptionMetadataTag.EXPERIMENTAL}, + help = + "Controls what Bazel does when a test action does not produce test.xml and split XML " + + "generation is enabled: 'legacy' silently generates XML with the TestRunner " + + "mnemonic, 'fail' fails the action, 'warn' reports a warning and generates XML " + + "with the distinct TestXmlGenerationFallback mnemonic, and 'workaround' wraps " + + "the test with a native XML fallback and fails if XML is still absent.") + public TestXmlMissingBehavior testXmlMissingBehavior; + @Option( name = "cache_test_results", defaultValue = "auto", @@ -416,6 +446,10 @@ public boolean getTestRunnerFailFast() { return options.testRunnerFailFast; } + public TestXmlMissingBehavior getTestXmlMissingBehavior() { + return options.testXmlMissingBehavior; + } + public TriState cacheTestResults() { return options.cacheTestResults; } 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 4f4fbc5c6400a8..467a2d2f27aeff 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 @@ -56,6 +56,7 @@ import com.google.devtools.build.lib.analysis.test.TestActionContext.TestAttemptResult; import com.google.devtools.build.lib.analysis.test.TestActionContext.TestAttemptResult.Result; import com.google.devtools.build.lib.analysis.test.TestActionContext.TestRunnerSpawn; +import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestXmlMissingBehavior; import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions.CancelConcurrentTests; import com.google.devtools.build.lib.buildeventstream.TestFileNameConstants; import com.google.devtools.build.lib.cmdline.Label; @@ -115,6 +116,7 @@ public class TestRunnerAction extends AbstractAction private final Artifact runfilesMiddleman; private final Artifact testSetupScript; private final Artifact testXmlGeneratorScript; + @Nullable private final Artifact ensureXmlExecutable; private final FilesToRunProvider collectCoverageScript; private final BuildConfigurationValue configuration; private final TestConfiguration testConfiguration; @@ -200,6 +202,7 @@ private static ImmutableSet nonNullAsSet(Artifact... artifacts) { Artifact runfilesMiddleman, Artifact testSetupScript, // Must be in inputs Artifact testXmlGeneratorScript, // Must be in inputs + @Nullable Artifact ensureXmlExecutable, // Must be in inputs, if not null @Nullable FilesToRunProvider collectCoverageScript, // filesToRun must be in input, if not null Artifact testLog, @@ -236,6 +239,7 @@ private static ImmutableSet nonNullAsSet(Artifact... artifacts) { this.runfilesMiddleman = runfilesMiddleman; this.testSetupScript = testSetupScript; this.testXmlGeneratorScript = testXmlGeneratorScript; + this.ensureXmlExecutable = ensureXmlExecutable; this.collectCoverageScript = collectCoverageScript; this.configuration = checkNotNull(configuration); this.testConfiguration = checkNotNull(configuration.getFragment(TestConfiguration.class)); @@ -515,6 +519,7 @@ protected void computeKey( fp.addIterableStrings(executionSettings.getArgs().arguments()); fp.addString(Strings.nullToEmpty(executionSettings.getTestFilter())); fp.addBoolean(executionSettings.getTestRunnerFailFast()); + fp.addString(testConfiguration.getTestXmlMissingBehavior().name()); RunUnder runUnder = executionSettings.getRunUnder(); fp.addString(runUnder == null ? "" : runUnder.value()); fp.addStringMap(coverageEnv); @@ -1081,6 +1086,15 @@ public Artifact getTestXmlGeneratorScript() { return testXmlGeneratorScript; } + @Nullable + public Artifact getEnsureXmlExecutable() { + return ensureXmlExecutable; + } + + public TestXmlMissingBehavior getTestXmlMissingBehavior() { + return testConfiguration.getTestXmlMissingBehavior(); + } + @Nullable public FilesToRunProvider getCollectCoverageScript() { return collectCoverageScript; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java index 2e76320d974cff..d91278b3fd4243 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestStrategy.java @@ -195,8 +195,13 @@ public final AttemptGroup getAttemptGroup(ActionOwner owner, int shardNum) { */ public static ImmutableList getArgs(TestRunnerAction testAction) throws ExecException, InterruptedException { + return getArgs(testAction, /* ensureXml= */ false); + } + + public static ImmutableList getArgs(TestRunnerAction testAction, boolean ensureXml) + throws ExecException, InterruptedException { try { - return expandedArgsFromAction(testAction); + return expandedArgsFromAction(testAction, ensureXml); } catch (CommandLineExpansionException e) { throw new UserExecException( e, @@ -217,6 +222,12 @@ public static ImmutableList getArgs(TestRunnerAction testAction) */ public static ImmutableList expandedArgsFromAction(TestRunnerAction testAction) throws CommandLineExpansionException, InterruptedException { + return expandedArgsFromAction(testAction, /* ensureXml= */ false); + } + + public static ImmutableList expandedArgsFromAction( + TestRunnerAction testAction, boolean ensureXml) + throws CommandLineExpansionException, InterruptedException { List args = Lists.newArrayList(); OS executionOs = testAction.getExecutionSettings().getExecutionOs(); @@ -234,6 +245,15 @@ public static ImmutableList expandedArgsFromAction(TestRunnerAction test TestTargetExecutionSettings execSettings = testAction.getExecutionSettings(); + if (ensureXml) { + Artifact ensureXmlExecutable = + Preconditions.checkNotNull(testAction.getEnsureXmlExecutable(), "%s", testAction); + args.add( + ensureXmlExecutable + .getExecPath() + .getCallablePathStringForOs(executionOs)); + } + // Insert the command prefix specified by the "--run_under=" option, if any. if (execSettings.getRunUnder() != null) { addRunUnderArgs(testAction, args); diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD index f898447f1d1054..63563f92fa9197 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD @@ -420,6 +420,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", + "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration", "//src/main/java/com/google/devtools/build/lib/buildeventstream", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", 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 fdefad3f2be658..0f7963bc965767 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 @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.TestExecException; import com.google.devtools.build.lib.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestXmlMissingBehavior; import com.google.devtools.build.lib.analysis.test.TestAttempt; import com.google.devtools.build.lib.analysis.test.TestResult; import com.google.devtools.build.lib.analysis.test.TestRunnerAction; @@ -47,6 +48,7 @@ import com.google.devtools.build.lib.buildeventstream.TestFileNameConstants; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.runtime.TestSummaryOptions; import com.google.devtools.build.lib.server.FailureDetails.Execution.Code; @@ -73,6 +75,7 @@ /** Runs TestRunnerAction actions. */ // TODO(bazel-team): add tests for this strategy. public class StandaloneTestStrategy extends TestStrategy { + private static final String XML_GENERATION_FALLBACK_MNEMONIC = "TestXmlGenerationFallback"; private static final ImmutableMap ENV_VARS = ImmutableMap.builder() .put("TZ", "UTC") @@ -105,6 +108,11 @@ public TestRunnerSpawn createTestRunnerSpawn( Map testEnvironment = createEnvironment( actionExecutionContext, action, tmpDirRoot, executionOptions.splitXmlGeneration); + boolean ensureXml = + executionOptions.splitXmlGeneration && action.getEnsureXmlExecutable() != null; + if (ensureXml) { + testEnvironment.put("EXPERIMENTAL_TEST_ENSURE_XML", "1"); + } Map executionInfo = new TreeMap<>(action.getExecutionInfo()); if (!action.shouldAcceptCachedResult()) { @@ -125,7 +133,7 @@ public TestRunnerSpawn createTestRunnerSpawn( Spawn spawn = new SimpleSpawn( action, - getArgs(action), + getArgs(action, ensureXml), ImmutableMap.copyOf(testEnvironment), ImmutableMap.copyOf(executionInfo), ImmutableMap.of(), @@ -440,7 +448,10 @@ private static Duration toProtoDuration(int timeInMs) { * generate a test.xml file itself. */ private static Spawn createXmlGeneratingSpawn( - TestRunnerAction action, ImmutableMap testEnv, SpawnResult result) { + TestRunnerAction action, + ImmutableMap testEnv, + SpawnResult result, + boolean useFallbackMnemonic) { ImmutableList args = ImmutableList.of( action @@ -462,16 +473,33 @@ private static Spawn createXmlGeneratingSpawn( envBuilder.put("TEST_SHARD_INDEX", "0"); envBuilder.put("TEST_TOTAL_SHARDS", "0"); } + ImmutableMap environment = envBuilder.buildOrThrow(); + var inputs = + NestedSetBuilder.create( + Order.STABLE_ORDER, action.getTestXmlGeneratorScript(), action.getTestLog()); + if (useFallbackMnemonic) { + return SimpleSpawn.withMnemonic( + action, + args, + environment, + action.getExecutionInfo(), + ImmutableMap.of(), + inputs, + /* tools= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER), + /* outputs= */ ImmutableSet.of(action.getTestXml()), + /* mandatoryOutputs= */ null, + SpawnAction.DEFAULT_RESOURCE_SET, + XML_GENERATION_FALLBACK_MNEMONIC); + } return new SimpleSpawn( action, args, - envBuilder.buildOrThrow(), + environment, // Pass the execution info of the action which is identical to the supported tags set on the // test target. In particular, this does not set the test timeout on the spawn. action.getExecutionInfo(), ImmutableMap.of(), - /* inputs= */ NestedSetBuilder.create( - Order.STABLE_ORDER, action.getTestXmlGeneratorScript(), action.getTestLog()), + inputs, /* tools= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER), /* outputs= */ ImmutableSet.of(action.getTestXml()), /* mandatoryOutputs= */ null, @@ -847,14 +875,29 @@ private TestAttemptResult runTestAttempt( Path xmlOutputPath = resolvedPaths.getXmlOutputPath(); // 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. We do this as a spawn - // rather than doing it locally in-process, as the test.log file may only exist remotely (when - // remote execution is enabled), and we do not want to have to download it. + // legacy and warn modes run a separate action to create a test.xml from test.log. We do this as + // a spawn rather than doing it locally in-process, as the test.log file may only exist remotely + // (when remote execution is enabled), and we do not want to have to download it. if (executionOptions.splitXmlGeneration && fileOutErr.getOutputPath().exists() && !xmlOutputPath.exists()) { + String missingXmlMessage = + String.format( + "Test action %s failed to produce test.xml", testAction.getOwner().getLabel()); + TestXmlMissingBehavior missingBehavior = testAction.getTestXmlMissingBehavior(); + if (missingBehavior == TestXmlMissingBehavior.FAIL + || missingBehavior == TestXmlMissingBehavior.WORKAROUND) { + throw createTestExecException(TestAction.Code.MISSING_XML_OUTPUT, missingXmlMessage); + } + if (missingBehavior == TestXmlMissingBehavior.WARN) { + actionExecutionContext.getEventHandler().handle(Event.warn(missingXmlMessage)); + } Spawn xmlGeneratingSpawn = - createXmlGeneratingSpawn(testAction, spawn.getEnvironment(), spawnResults.get(0)); + createXmlGeneratingSpawn( + testAction, + spawn.getEnvironment(), + spawnResults.get(0), + missingBehavior == TestXmlMissingBehavior.WARN); SpawnStrategyResolver spawnStrategyResolver = actionExecutionContext.getContext(SpawnStrategyResolver.class); // We treat all failures to generate the test.xml here as catastrophic, and won't rerun diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index d77aded3e134c2..2642e0865bfd59 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -1209,6 +1209,7 @@ message TestAction { COMMAND_LINE_EXPANSION_FAILURE = 3 [(metadata) = { exit_code: 1 }]; DUPLICATE_CPU_TAGS = 4 [(metadata) = { exit_code: 1 }]; INVALID_CPU_TAG = 5 [(metadata) = { exit_code: 1 }]; + MISSING_XML_OUTPUT = 6 [(metadata) = { exit_code: 1 }]; } Code code = 1; 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 ae6b3f5f045a4f..01b9768dc2801e 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 @@ -417,6 +417,15 @@ def _impl(ctx): srcs = ["xml_writer_bin"], ) + filegroup( + name = "ensure_xml", + srcs = [ + "ensure_xml_darwin_arm64", + "ensure_xml_linux_amd64", + "ensure_xml_linux_arm64", + ], + ) + filegroup( name = "test_setup", srcs = ["test-setup.sh"], @@ -490,9 +499,14 @@ function rlocation() { else TEST_PATH="$(rlocation $TEST_WORKSPACE/$EXE)" fi - exec $TEST_PATH + exec "$TEST_PATH" "$@" """) .chmod(0755); + for (String platform : ImmutableList.of("darwin_arm64", "linux_amd64", "linux_arm64")) { + config + .create("embedded_tools/tools/test/ensure_xml_" + platform, "#!/bin/sh", "exec \"$@\"") + .chmod(0755); + } config .create("embedded_tools/tools/test/test-xml-generator.sh", "#!/bin/sh", "cp \"$1\" \"$2\"") .chmod(0755); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/test/TestActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/test/TestActionBuilderTest.java index f93c994943e26a..dfdf56476004f9 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/test/TestActionBuilderTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/test/TestActionBuilderTest.java @@ -760,6 +760,124 @@ private ImmutableList getTestStatusArtifacts(String la return target.getProvider(TestProvider.class).getTestParams().getTestStatusArtifacts(); } + @Test + public void testEnsureXmlWrapsRunUnderAndTest() throws Exception { + scratch.file( + "xml_wrapper/BUILD", + "load('//test_defs:foo_test.bzl', 'foo_test')", + "genrule(", + " name = 'run_under',", + " outs = ['run_under.sh'],", + " cmd = \"echo '#!/bin/sh' > $@\",", + " executable = True,", + ")", + "foo_test(", + " name = 'test',", + " srcs = ['test.sh'],", + ")"); + scratch.file("xml_wrapper/test.sh", "#!/bin/sh", "exit 0"); + useConfiguration( + "--experimental_test_xml_missing_behavior=workaround", + "--run_under=//xml_wrapper:run_under"); + + TestRunnerAction testAction = + (TestRunnerAction) + getGeneratingAction(getTestStatusArtifacts("//xml_wrapper:test").get(0)); + Artifact ensureXml = testAction.getEnsureXmlExecutable(); + assertThat(ensureXml).isNotNull(); + assertThat(testAction.getInputs().toList()).contains(ensureXml); + + ImmutableList args = + TestStrategy.expandedArgsFromAction(testAction, /* ensureXml= */ true); + assertThat(PathFragment.create(args.get(1)).getBaseName()).startsWith("ensure_xml_"); + assertThat(PathFragment.create(args.get(2)).getBaseName()).isEqualTo("run_under.sh"); + assertThat(PathFragment.create(args.get(3)).getBaseName()).isEqualTo("test"); + } + + @Test + public void testEnsureXmlIsNotInputOutsideWorkaroundMode() throws Exception { + useConfiguration("--experimental_test_xml_missing_behavior=warn"); + + TestRunnerAction testAction = + (TestRunnerAction) getGeneratingAction(getTestStatusArtifacts("//tests:small_test_2").get(0)); + + assertThat(testAction.getEnsureXmlExecutable()).isNull(); + assertThat( + testAction.getInputs().toList().stream() + .map(artifact -> artifact.getExecPath().getBaseName()) + .anyMatch(name -> name.startsWith("ensure_xml_"))) + .isFalse(); + } + + @Test + public void testEnsureXmlUsesTargetPlatformForTests() throws Exception { + scratch.file( + "xml_platform/BUILD", + "load('//test_defs:foo_test.bzl', 'foo_test')", + "platform(", + " name = 'linux_aarch64',", + " constraint_values = [", + " '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:linux',", + " '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:aarch64',", + " ],", + ")", + "foo_test(name = 'test', srcs = ['test.sh'])"); + scratch.file("xml_platform/test.sh", "#!/bin/sh", "exit 0"); + useConfiguration( + "--experimental_test_xml_missing_behavior=workaround", + "--use_target_platform_for_tests=true", + "--platforms=//xml_platform:linux_aarch64"); + + TestRunnerAction testAction = + (TestRunnerAction) + getGeneratingAction(getTestStatusArtifacts("//xml_platform:test").get(0)); + + assertThat(testAction.getExecutionPlatform().label().getName()).isEqualTo("linux_aarch64"); + assertThat(testAction.getEnsureXmlExecutable().getFilename()) + .isEqualTo("ensure_xml_linux_arm64"); + assertThat( + testAction.getInputs().toList().stream() + .map(artifact -> artifact.getExecPath().getBaseName()) + .filter(name -> name.startsWith("ensure_xml_"))) + .containsExactly("ensure_xml_linux_arm64"); + } + + @Test + public void testEnsureXmlIsOmittedForUnsupportedExecutionPlatform() throws Exception { + scratch.file( + "xml_platform/BUILD", + "load('//test_defs:foo_test.bzl', 'foo_test')", + "platform(", + " name = 'darwin_x86_64',", + " constraint_values = [", + " '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:osx',", + " '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:x86_64',", + " ],", + ")", + "foo_test(name = 'test', srcs = ['test.sh'])"); + scratch.file("xml_platform/test.sh", "#!/bin/sh", "exit 0"); + useConfiguration( + "--experimental_test_xml_missing_behavior=workaround", + "--use_target_platform_for_tests=true", + "--platforms=//xml_platform:darwin_x86_64"); + + TestRunnerAction testAction = + (TestRunnerAction) + getGeneratingAction(getTestStatusArtifacts("//xml_platform:test").get(0)); + + assertThat(testAction.getExecutionPlatform().label().getName()).isEqualTo("darwin_x86_64"); + assertThat(testAction.getEnsureXmlExecutable()).isNull(); + } + + @Test + public void testMissingXmlBehaviorDefaultsToLegacy() throws Exception { + TestRunnerAction testAction = + (TestRunnerAction) getGeneratingAction(getTestStatusArtifacts("//tests:small_test_2").get(0)); + + assertThat(testAction.getTestXmlMissingBehavior()) + .isEqualTo(TestConfiguration.TestXmlMissingBehavior.LEGACY); + } + @Test public void testRunUnderConfiguredForTestExecPlatform() throws Exception { scratch.file( 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 8aed7a7033f488..9db5a521557b62 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 @@ -17,7 +17,9 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.truth.Truth.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -48,6 +50,7 @@ import com.google.devtools.build.lib.analysis.test.TestActionContext.TestRunnerSpawn; import com.google.devtools.build.lib.analysis.test.TestAttempt; import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions.CancelConcurrentTests; +import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestXmlMissingBehavior; import com.google.devtools.build.lib.analysis.test.TestProvider; import com.google.devtools.build.lib.analysis.test.TestResult; import com.google.devtools.build.lib.analysis.test.TestRunnerAction; @@ -73,6 +76,7 @@ import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; import com.google.devtools.build.lib.view.test.TestStatus.TestResultData; @@ -663,7 +667,13 @@ public void testThatTestLogAndOutputAreReturned() throws Exception { } @Test - public void testThatTestLogAndOutputAreReturnedWithSplitXmlGeneration() throws Exception { + public void testThatTestLogAndOutputAreReturnedWithSplitXmlGeneration( + @TestParameter({"LEGACY", "WARN"}) + TestXmlMissingBehavior testXmlMissingBehavior) + throws Exception { + useConfiguration( + "--experimental_test_xml_missing_behavior=" + + (testXmlMissingBehavior == TestXmlMissingBehavior.WARN ? "warn" : "legacy")); ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class); TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class); executionOptions.testOutput = ExecutionOptions.TestOutputFormat.ERRORS; @@ -713,6 +723,11 @@ public void testThatTestLogAndOutputAreReturnedWithSplitXmlGeneration() throws E /* forciblyRunRemotely= */ false, /* catastrophe= */ false); } else { + assertThat(spawn.getMnemonic()) + .isEqualTo( + testXmlMissingBehavior == TestXmlMissingBehavior.WARN + ? "TestXmlGenerationFallback" + : "TestRunner"); String testName = "standalone/failing_test"; assertThat(spawn.getEnvironment()).containsEntry("TEST_BINARY", testName); return ImmutableList.of(xmlGeneratorSpawnResult); @@ -749,6 +764,160 @@ public void testThatTestLogAndOutputAreReturnedWithSplitXmlGeneration() throws E assertThat(outErr.getErrorPath().exists()).isFalse(); assertThat(called).hasSize(2); assertThat(called).containsNoDuplicates(); + assertThat( + storedEvents.getEvents().stream() + .anyMatch( + event -> + event.getKind() == EventKind.WARNING + && event.getMessage().contains("failed to produce test.xml"))) + .isEqualTo(testXmlMissingBehavior == TestXmlMissingBehavior.WARN); + } + + @Test + public void missingXmlStrictBehaviorFailsWithoutGeneratingXml( + @TestParameter({"FAIL", "WORKAROUND"}) + TestXmlMissingBehavior testXmlMissingBehavior) + throws Exception { + useConfiguration( + "--experimental_test_xml_missing_behavior=" + + (testXmlMissingBehavior == TestXmlMissingBehavior.FAIL ? "fail" : "workaround")); + ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class); + executionOptions.splitXmlGeneration = true; + TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class); + Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions); + BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools()); + TestedStandaloneTestStrategy standaloneTestStrategy = + new TestedStandaloneTestStrategy( + executionOptions, testSummaryOptions, binTools, tmpDirRoot); + + scratch.file("standalone/missing_xml_test.sh", "this does not get executed, it is mocked out"); + scratch.file( + "standalone/BUILD", + """ + load('//test_defs:foo_test.bzl', 'foo_test') + foo_test( + name = "missing_xml_test", + size = "small", + srcs = ["missing_xml_test.sh"], + ) + """); + TestRunnerAction testRunnerAction = getTestAction("//standalone:missing_xml_test"); + when(spawnStrategy.exec(any(), any())).thenReturn(ImmutableList.of(PASSED_TEST_SPAWN)); + ActionExecutionContext actionExecutionContext = + new FakeActionExecutionContext( + createTempOutErr(tmpDirRoot), + inputMetadataFor(testRunnerAction), + spawnStrategy, + binTools); + + ActionExecutionException exception = + assertThrows( + ActionExecutionException.class, + () -> execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy)); + + assertThat(exception).hasMessageThat().contains("failed to produce test.xml"); + verify(spawnStrategy, times(1)).exec(any(), any()); + } + + @Test + public void workaroundWrapperProducesXmlWithoutDerivedSpawn() throws Exception { + useConfiguration("--experimental_test_xml_missing_behavior=workaround"); + ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class); + executionOptions.splitXmlGeneration = true; + TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class); + Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions); + BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools()); + TestedStandaloneTestStrategy standaloneTestStrategy = + new TestedStandaloneTestStrategy( + executionOptions, testSummaryOptions, binTools, tmpDirRoot); + + scratch.file("standalone/wrapped_test.sh", "this does not get executed, it is mocked out"); + scratch.file( + "standalone/BUILD", + """ + load('//test_defs:foo_test.bzl', 'foo_test') + foo_test( + name = "wrapped_test", + size = "small", + srcs = ["wrapped_test.sh"], + ) + """); + TestRunnerAction testRunnerAction = getTestAction("//standalone:wrapped_test"); + ActionExecutionContext actionExecutionContext = + new FakeActionExecutionContext( + createTempOutErr(tmpDirRoot), + inputMetadataFor(testRunnerAction), + spawnStrategy, + binTools); + when(spawnStrategy.exec(any(), any())) + .thenAnswer( + invocation -> { + Spawn spawn = invocation.getArgument(0); + assertThat( + spawn.getArguments().stream() + .map(argument -> PathFragment.create(argument).getBaseName()) + .anyMatch(name -> name.startsWith("ensure_xml_"))) + .isTrue(); + assertThat(spawn.getEnvironment()) + .containsEntry("EXPERIMENTAL_TEST_ENSURE_XML", "1"); + FileSystemUtils.writeContent( + actionExecutionContext.getInputPath(testRunnerAction.getTestXml()), + UTF_8, + "\n"); + return ImmutableList.of(PASSED_TEST_SPAWN); + }); + assertThat(execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy)) + .containsExactly(PASSED_TEST_SPAWN); + verify(spawnStrategy, times(1)).exec(any(), any()); + } + + @Test + public void workaroundWrapperIsDisabledWithoutSplitXmlGeneration() throws Exception { + useConfiguration("--experimental_test_xml_missing_behavior=workaround"); + ExecutionOptions executionOptions = Options.getDefaults(ExecutionOptions.class); + executionOptions.splitXmlGeneration = false; + TestSummaryOptions testSummaryOptions = Options.getDefaults(TestSummaryOptions.class); + Path tmpDirRoot = TestStrategy.getTmpRoot(rootDirectory, outputBase, executionOptions); + BinTools binTools = BinTools.forUnitTesting(directories, analysisMock.getEmbeddedTools()); + TestedStandaloneTestStrategy standaloneTestStrategy = + new TestedStandaloneTestStrategy( + executionOptions, testSummaryOptions, binTools, tmpDirRoot); + + scratch.file("standalone/unwrapped_test.sh", "this does not get executed, it is mocked out"); + scratch.file( + "standalone/BUILD", + """ + load('//test_defs:foo_test.bzl', 'foo_test') + foo_test( + name = "unwrapped_test", + size = "small", + srcs = ["unwrapped_test.sh"], + ) + """); + TestRunnerAction testRunnerAction = getTestAction("//standalone:unwrapped_test"); + when(spawnStrategy.exec(any(), any())) + .thenAnswer( + invocation -> { + Spawn spawn = invocation.getArgument(0); + assertThat( + spawn.getArguments().stream() + .map(argument -> PathFragment.create(argument).getBaseName()) + .noneMatch(name -> name.startsWith("ensure_xml_"))) + .isTrue(); + assertThat(spawn.getEnvironment()).doesNotContainKey("EXPERIMENTAL_TEST_ENSURE_XML"); + return ImmutableList.of(PASSED_TEST_SPAWN); + }); + + ActionExecutionContext actionExecutionContext = + new FakeActionExecutionContext( + createTempOutErr(tmpDirRoot), + inputMetadataFor(testRunnerAction), + spawnStrategy, + binTools); + + assertThat(execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy)) + .containsExactly(PASSED_TEST_SPAWN); + verify(spawnStrategy, times(1)).exec(any(), any()); } @Test diff --git a/tools/test/BUILD b/tools/test/BUILD index 95c61dbb6683c7..a5e424decf1c76 100644 --- a/tools/test/BUILD +++ b/tools/test/BUILD @@ -104,7 +104,9 @@ cc_test( filegroup( name = "srcs", - srcs = glob(["**"]), + srcs = glob(["**"]) + [ + "//tools/test/ensure_xml:srcs", + ], ) filegroup( @@ -113,6 +115,7 @@ filegroup( "BUILD.tools", "collect_cc_coverage.sh", "collect_coverage.sh", + "//tools/test/ensure_xml:embedded_files", "extensions.bzl", "generate-xml.sh", "test-setup.sh", diff --git a/tools/test/BUILD.tools b/tools/test/BUILD.tools index 804c5751e12eb9..e68a74de280e55 100644 --- a/tools/test/BUILD.tools +++ b/tools/test/BUILD.tools @@ -68,6 +68,15 @@ filegroup( }), ) +filegroup( + name = "ensure_xml", + srcs = [ + "ensure_xml/ensure_xml_darwin_arm64", + "ensure_xml/ensure_xml_linux_amd64", + "ensure_xml/ensure_xml_linux_arm64", + ], +) + filegroup( name = "bzl_srcs", srcs = ["extensions.bzl"], diff --git a/tools/test/ensure_xml/BUILD b/tools/test/ensure_xml/BUILD new file mode 100644 index 00000000000000..cba7080881ba0e --- /dev/null +++ b/tools/test/ensure_xml/BUILD @@ -0,0 +1,59 @@ +load("@rules_go//go:def.bzl", "go_binary", "go_cross_binary", "go_library", "go_test") + +package(default_visibility = ["//visibility:private"]) + +filegroup( + name = "srcs", + srcs = glob(["*"]), + visibility = ["//tools/test:__pkg__"], +) + +filegroup( + name = "embedded_files", + srcs = [ + "ensure_xml_darwin_arm64", + "ensure_xml_linux_amd64", + "ensure_xml_linux_arm64", + ], + visibility = ["//tools/test:__pkg__"], +) + +go_library( + name = "ensure_xml_lib", + srcs = ["main.go"], + importpath = "github.com/bazelbuild/bazel/tools/test/ensure_xml", + visibility = ["//visibility:private"], +) + +go_binary( + name = "ensure_xml", + embed = [":ensure_xml_lib"], + gc_linkopts = [ + "-s", + "-w", + ], + pure = "on", +) + +go_test( + name = "ensure_xml_test", + srcs = ["main_test.go"], + embed = [":ensure_xml_lib"], + pure = "on", +) + +[ + go_cross_binary( + name = "ensure_xml_" + platform + "_build", + platform = "@rules_go//go/toolchain:" + platform, + target = ":ensure_xml", + ) + for platform in [ + "darwin_amd64", + "darwin_arm64", + "linux_amd64", + "linux_arm64", + "windows_amd64", + "windows_arm64", + ] +] diff --git a/tools/test/ensure_xml/README.md b/tools/test/ensure_xml/README.md new file mode 100644 index 00000000000000..74ed58d59274a8 --- /dev/null +++ b/tools/test/ensure_xml/README.md @@ -0,0 +1,30 @@ +# Test XML fallback wrapper + +`ensure_xml` supervises a test command. If that command exits without creating +`$XML_OUTPUT_FILE`, the wrapper writes a minimal JUnit XML result and preserves +the command's exit status. It never replaces XML produced by the test. + +The root cause remains a test runner that failed to honor Bazel's declared XML +output contract. This wrapper is an opt-in compatibility workaround for such +tests. Workaround mode fails if the wrapper cannot publish XML; it does not hide +that failure behind Bazel's derived XML action. + +The Linux executables are checked-in embedded tools because they must be action +inputs before an execution platform has a Go toolchain, and remote executors +should not need a JRE or libc. Regenerate them from source with: + +```sh +bazel build //tools/test/ensure_xml:ensure_xml_linux_amd64_build +cp bazel-bin/tools/test/ensure_xml/ensure_xml_linux_amd64_build \ + tools/test/ensure_xml/ensure_xml_linux_amd64 +bazel build //tools/test/ensure_xml:ensure_xml_linux_arm64_build +cp bazel-bin/tools/test/ensure_xml/ensure_xml_linux_arm64_build \ + tools/test/ensure_xml/ensure_xml_linux_arm64 +bazel build //tools/test/ensure_xml:ensure_xml_darwin_arm64_build +cp bazel-bin/tools/test/ensure_xml/ensure_xml_darwin_arm64_build \ + tools/test/ensure_xml/ensure_xml_darwin_arm64 +``` + +Other Unix platforms and Windows omit the supervisor. On those platforms, +workaround mode succeeds only when the test itself writes XML. Otherwise Bazel +reports a missing-output error. diff --git a/tools/test/ensure_xml/ensure_xml_darwin_arm64 b/tools/test/ensure_xml/ensure_xml_darwin_arm64 new file mode 100755 index 00000000000000..a87df6c7670003 Binary files /dev/null and b/tools/test/ensure_xml/ensure_xml_darwin_arm64 differ diff --git a/tools/test/ensure_xml/ensure_xml_linux_amd64 b/tools/test/ensure_xml/ensure_xml_linux_amd64 new file mode 100755 index 00000000000000..7a50e9dbdd2892 Binary files /dev/null and b/tools/test/ensure_xml/ensure_xml_linux_amd64 differ diff --git a/tools/test/ensure_xml/ensure_xml_linux_arm64 b/tools/test/ensure_xml/ensure_xml_linux_arm64 new file mode 100755 index 00000000000000..776515f0ea3196 Binary files /dev/null and b/tools/test/ensure_xml/ensure_xml_linux_arm64 differ diff --git a/tools/test/ensure_xml/main.go b/tools/test/ensure_xml/main.go new file mode 100644 index 00000000000000..714998bed69bb7 --- /dev/null +++ b/tools/test/ensure_xml/main.go @@ -0,0 +1,210 @@ +// Copyright 2026 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. + +// ensure_xml supervises a Bazel test command and creates a minimal JUnit XML +// result when the test runner does not create one itself. +package main + +import ( + "encoding/xml" + "errors" + "fmt" + "io" + "os" + "os/exec" + "os/signal" + "path/filepath" + "strconv" + "sync" + "syscall" + "time" +) + +type testSuites struct { + XMLName xml.Name `xml:"testsuites"` + Suite testSuite `xml:"testsuite"` +} + +type testSuite struct { + Name string `xml:"name,attr"` + Tests int `xml:"tests,attr"` + Failures int `xml:"failures,attr"` + Errors int `xml:"errors,attr"` + Time string `xml:"time,attr"` + Case testCase `xml:"testcase"` +} + +type testCase struct { + Name string `xml:"name,attr"` + Status string `xml:"status,attr"` + Time string `xml:"time,attr"` + Error *testError `xml:"error,omitempty"` +} + +type testError struct { + Message string `xml:"message,attr"` +} + +type childResult struct { + exitCode int + message string +} + +func main() { + os.Exit(supervise(os.Args[1:], os.Stderr)) +} + +func supervise(args []string, stderr io.Writer) int { + start := time.Now() + result := childResult{exitCode: 127, message: "test command was not provided"} + + if len(args) > 0 { + command := exec.Command(args[0], args[1:]...) + command.Stdin = os.Stdin + command.Stdout = os.Stdout + command.Stderr = os.Stderr + + if err := command.Start(); err != nil { + result.message = fmt.Sprintf("could not start test: %v", err) + } else { + result = waitForChild(command) + } + } + + if err := ensureXML(os.Getenv("XML_OUTPUT_FILE"), testName(), time.Since(start), result); err != nil { + fmt.Fprintf(stderr, "ensure_xml: %v\n", err) + } + return result.exitCode +} + +func waitForChild(command *exec.Cmd) childResult { + signals := make(chan os.Signal, 4) + done := make(chan struct{}) + signal.Notify(signals, os.Interrupt, syscall.SIGTERM) + defer signal.Stop(signals) + + var lastSignal os.Signal + var mu sync.Mutex + go func() { + for { + select { + case received := <-signals: + mu.Lock() + lastSignal = received + mu.Unlock() + // The test setup normally signals the whole process group. Forwarding + // explicitly also handles runners that signal only this wrapper. + _ = command.Process.Signal(received) + case <-done: + return + } + } + }() + + err := command.Wait() + close(done) + if err == nil { + return childResult{exitCode: 0} + } + + var exitError *exec.ExitError + if errors.As(err, &exitError) { + code := exitError.ExitCode() + if code >= 0 { + return childResult{exitCode: code, message: fmt.Sprintf("test exited with code %d", code)} + } + mu.Lock() + defer mu.Unlock() + if signalNumber, ok := lastSignal.(syscall.Signal); ok { + code = 128 + int(signalNumber) + return childResult{ + exitCode: code, + message: fmt.Sprintf("test terminated by signal %s", lastSignal), + } + } + return childResult{exitCode: 1, message: "test terminated by signal"} + } + return childResult{exitCode: 1, message: fmt.Sprintf("could not wait for test: %v", err)} +} + +func testName() string { + if target := os.Getenv("TEST_TARGET"); target != "" { + return target + } + if binary := os.Getenv("TEST_BINARY"); binary != "" { + return binary + } + return "test" +} + +func ensureXML(path string, name string, duration time.Duration, result childResult) error { + if path == "" { + return errors.New("XML_OUTPUT_FILE is not set") + } + if _, err := os.Stat(path); err == nil { + return nil + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("could not inspect %s: %w", path, err) + } + + timeInSeconds := strconv.FormatFloat(duration.Seconds(), 'f', 3, 64) + suite := testSuite{ + Name: name, + Tests: 1, + Time: timeInSeconds, + Case: testCase{ + Name: name, + Status: "run", + Time: timeInSeconds, + }, + } + if result.exitCode != 0 { + suite.Errors = 1 + suite.Case.Error = &testError{Message: result.message} + } + + contents, err := xml.MarshalIndent(testSuites{Suite: suite}, "", " ") + if err != nil { + return fmt.Errorf("could not encode test XML: %w", err) + } + contents = append([]byte(xml.Header), append(contents, '\n')...) + + directory := filepath.Dir(path) + temporary, err := os.CreateTemp(directory, ".test.xml.*") + if err != nil { + return fmt.Errorf("could not create temporary XML: %w", err) + } + temporaryPath := temporary.Name() + defer os.Remove(temporaryPath) + + if _, err := temporary.Write(contents); err != nil { + temporary.Close() + return fmt.Errorf("could not write temporary XML: %w", err) + } + if err := temporary.Close(); err != nil { + return fmt.Errorf("could not close temporary XML: %w", err) + } + + // The child has exited, but recheck so an XML file it produced is never + // replaced if it became visible after the first stat. + if _, err := os.Stat(path); err == nil { + return nil + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("could not recheck %s: %w", path, err) + } + if err := os.Rename(temporaryPath, path); err != nil { + return fmt.Errorf("could not publish test XML: %w", err) + } + return nil +} diff --git a/tools/test/ensure_xml/main_test.go b/tools/test/ensure_xml/main_test.go new file mode 100644 index 00000000000000..3be025e60acc29 --- /dev/null +++ b/tools/test/ensure_xml/main_test.go @@ -0,0 +1,116 @@ +// Copyright 2026 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. + +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestEnsureXMLForPassingTest(t *testing.T) { + path := filepath.Join(t.TempDir(), "test.xml") + if err := ensureXML( + path, "//pkg:test", 1234*time.Millisecond, childResult{exitCode: 0}); err != nil { + t.Fatal(err) + } + + contents, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + got := string(contents) + for _, want := range []string{ + ``, + ``, + } { + if !strings.Contains(got, want) { + t.Errorf("XML did not contain %q:\n%s", want, got) + } + } +} + +func TestEnsureXMLForFailingTest(t *testing.T) { + path := filepath.Join(t.TempDir(), "test.xml") + if err := ensureXML( + path, + `//pkg:test&special`, + 2*time.Second, + childResult{exitCode: 23, message: `test exited with code 23 & failed`}, + ); err != nil { + t.Fatal(err) + } + + contents, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + got := string(contents) + for _, want := range []string{ + `name="//pkg:test&special"`, + `errors="1"`, + ``, + } { + if !strings.Contains(got, want) { + t.Errorf("XML did not contain %q:\n%s", want, got) + } + } +} + +func TestEnsureXMLDoesNotReplaceTestOutput(t *testing.T) { + path := filepath.Join(t.TempDir(), "test.xml") + const original = "\n" + if err := os.WriteFile(path, []byte(original), 0o644); err != nil { + t.Fatal(err) + } + if err := ensureXML(path, "fallback", time.Second, childResult{}); err != nil { + t.Fatal(err) + } + + contents, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if got := string(contents); got != original { + t.Fatalf("existing XML replaced: got %q, want %q", got, original) + } +} + +func TestSuperviseRunsChildAndCreatesXML(t *testing.T) { + path := filepath.Join(t.TempDir(), "test.xml") + t.Setenv("XML_OUTPUT_FILE", path) + t.Setenv("TEST_TARGET", "//pkg:supervised") + + if got := supervise([]string{"/bin/sh", "-c", "exit 7"}, os.Stderr); got != 7 { + t.Fatalf("supervise exit code = %d, want 7", got) + } + + contents, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + got := string(contents) + for _, want := range []string{ + `name="//pkg:supervised"`, + `errors="1"`, + `message="test exited with code 7"`, + } { + if !strings.Contains(got, want) { + t.Errorf("XML did not contain %q:\n%s", want, got) + } + } +} diff --git a/tools/test/test-setup.sh b/tools/test/test-setup.sh index 5d25b7c41e7be3..8dcd71d04cd582 100755 --- a/tools/test/test-setup.sh +++ b/tools/test/test-setup.sh @@ -144,6 +144,19 @@ if [ ! -z "$TEST_WORKSPACE" ]; then fi [[ -n "$RUNTEST_PRESERVE_CWD" ]] && DIR="$PWD" +# The ensure-XML supervisor is an embedded tool and therefore an action input, +# not a runfile of the test. Resolve it against the execroot before changing to +# the runfiles directory, then remove it from the normal test command line. +if [[ "${EXPERIMENTAL_TEST_ENSURE_XML:-}" == "1" ]]; then + if [ -z "$COVERAGE_DIR" ]; then + ENSURE_XML_PATH="$1" + shift + else + ENSURE_XML_PATH="$2" + set -- "$1" "${@:3}" + fi + is_absolute "$ENSURE_XML_PATH" || ENSURE_XML_PATH="$PWD/$ENSURE_XML_PATH" +fi # normal commands are run in the exec-root where they have access to # the entire source tree. By chdir'ing to the runfiles root, tests only @@ -332,7 +345,11 @@ start=$(date +%s) # Aren't processes fun? set -m if [[ "${EXPERIMENTAL_SPLIT_XML_GENERATION}" == "1" ]]; then - if [ -z "$COVERAGE_DIR" ]; then + if [[ "${EXPERIMENTAL_TEST_ENSURE_XML:-}" == "1" && -z "$COVERAGE_DIR" ]]; then + ("$ENSURE_XML_PATH" "${TEST_PATH}" "$@" 2>&1) <&0 & + elif [[ "${EXPERIMENTAL_TEST_ENSURE_XML:-}" == "1" ]]; then + ("$ENSURE_XML_PATH" "$1" "$TEST_PATH" "${@:3}" 2>&1) <&0 & + elif [ -z "$COVERAGE_DIR" ]; then ("${TEST_PATH}" "$@" 2>&1) <&0 & else ("$1" "$TEST_PATH" "${@:3}" 2>&1) <&0 &