Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
@Immutable
public final class SimpleSpawn implements Spawn {
private final ActionExecutionMetadata owner;
private final String mnemonic;
private final ImmutableList<String> arguments;
private final ImmutableMap<String, String> environment;
private final ImmutableMap<String, String> executionInfo;
Expand All @@ -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<String> arguments,
ImmutableMap<String, String> environment,
ImmutableMap<String, String> executionInfo,
ImmutableMap<Artifact, FilesetOutputTree> filesetMappings,
NestedSet<? extends ActionInput> inputs,
NestedSet<? extends ActionInput> tools,
Collection<? extends ActionInput> outputs,
@Nullable final Set<? extends ActionInput> 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);
Expand All @@ -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<String> arguments,
ImmutableMap<String, String> environment,
ImmutableMap<String, String> executionInfo,
ImmutableMap<Artifact, FilesetOutputTree> filesetMappings,
NestedSet<? extends ActionInput> inputs,
NestedSet<? extends ActionInput> tools,
Collection<? extends ActionInput> outputs,
@Nullable Set<? extends ActionInput> 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<String> arguments,
Expand Down Expand Up @@ -292,7 +354,7 @@ public PathMapper getPathMapper() {

@Override
public String getMnemonic() {
return owner.getMnemonic();
return mnemonic;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Artifact> additionalTools;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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<String, String> coverageTestEnv = new TreeMap<>();
Expand Down Expand Up @@ -356,8 +421,6 @@ private TestParams createTestAction()
ImmutableList.Builder<Artifact> coverageArtifacts = ImmutableList.builder();
ImmutableList.Builder<ActionInput> testOutputs = ImmutableList.builder();

ActionOwner actionOwner = getTestActionOwner(testConfiguration.useTargetPlatformForTests());

// Use 1-based indices for user friendliness.
for (int shard = 0; shard < shardRuns; shard++) {
String shardDir =
Expand Down Expand Up @@ -416,6 +479,7 @@ private TestParams createTestAction()
runfilesSupport.getRunfilesMiddleman(),
testActionExecutable,
testXmlGeneratorExecutable,
ensureXmlExecutable,
collectCoverageScript,
testLog,
testXml,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<TestXmlMissingBehavior> {
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)) {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -416,6 +446,10 @@ public boolean getTestRunnerFailFast() {
return options.testRunnerFailFast;
}

public TestXmlMissingBehavior getTestXmlMissingBehavior() {
return options.testXmlMissingBehavior;
}

public TriState cacheTestResults() {
return options.cacheTestResults;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -200,6 +202,7 @@ private static ImmutableSet<Artifact> 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,
Expand Down Expand Up @@ -236,6 +239,7 @@ private static ImmutableSet<Artifact> 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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,13 @@ public final AttemptGroup getAttemptGroup(ActionOwner owner, int shardNum) {
*/
public static ImmutableList<String> getArgs(TestRunnerAction testAction)
throws ExecException, InterruptedException {
return getArgs(testAction, /* ensureXml= */ false);
}

public static ImmutableList<String> getArgs(TestRunnerAction testAction, boolean ensureXml)
throws ExecException, InterruptedException {
try {
return expandedArgsFromAction(testAction);
return expandedArgsFromAction(testAction, ensureXml);
} catch (CommandLineExpansionException e) {
throw new UserExecException(
e,
Expand All @@ -217,6 +222,12 @@ public static ImmutableList<String> getArgs(TestRunnerAction testAction)
*/
public static ImmutableList<String> expandedArgsFromAction(TestRunnerAction testAction)
throws CommandLineExpansionException, InterruptedException {
return expandedArgsFromAction(testAction, /* ensureXml= */ false);
}

public static ImmutableList<String> expandedArgsFromAction(
TestRunnerAction testAction, boolean ensureXml)
throws CommandLineExpansionException, InterruptedException {
List<String> args = Lists.newArrayList();
OS executionOs = testAction.getExecutionSettings().getExecutionOs();

Expand All @@ -234,6 +245,15 @@ public static ImmutableList<String> 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=<command-prefix>" option, if any.
if (execSettings.getRunUnder() != null) {
addRunUnderArgs(testAction, args);
Expand Down
Loading
Loading