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 @@ -62,6 +62,14 @@
is enabled.
</li>

<li><code>requires-test-xml-generation</code> marks test actions as requiring
Bazel to ensure that they produce test XML. With
<code>--experimental_test_xml_missing_behavior=workaround</code> and split
XML generation enabled, Bazel supervises these test actions and generates
missing XML in the test spawn, as it does when
<code>--noexperimental_split_xml_generation</code> is set.
</li>

<li><code>block-network</code> keyword blocks access to the external
network from inside the sandbox. In this case, only communication
with localhost is allowed. This tag only has an effect if sandboxing is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,9 @@ public enum WorkerProtocolFormat {
*/
public static final String REQUIRES_NETWORK = "requires-network";

/** Indicates that test actions need Bazel to ensure that they produce test XML. */
public static final String REQUIRES_TEST_XML_GENERATION = "requires-test-xml-generation";

/**
* Disables networking for a spawn if possible (only if sandboxing is enabled and if the sandbox
* supports it).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.analysis.Allowlist;
import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
Expand Down Expand Up @@ -305,7 +306,8 @@ private TestParams createTestAction()
inputsBuilder.add(testXmlGeneratorExecutable);
Artifact ensureXmlExecutable = null;
if (testConfiguration.getTestXmlMissingBehavior()
== TestConfiguration.TestXmlMissingBehavior.WORKAROUND) {
== TestConfiguration.TestXmlMissingBehavior.WORKAROUND
&& testProperties.getTags().contains(ExecutionRequirements.REQUIRES_TEST_XML_GENERATION)) {
ensureXmlExecutable = getEnsureXmlExecutable(actionOwner);
if (ensureXmlExecutable != null) {
inputsBuilder.add(ensureXmlExecutable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,12 @@ private TestAttemptResult runTestAttempt(
String.format(
"Test action %s failed to produce test.xml", testAction.getOwner().getLabel());
TestXmlMissingBehavior missingBehavior = testAction.getTestXmlMissingBehavior();
if (missingBehavior == TestXmlMissingBehavior.WORKAROUND) {
missingXmlMessage +=
String.format(
"; add '%s' to the test target's tags attribute",
ExecutionRequirements.REQUIRES_TEST_XML_GENERATION);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misleading missing-XML tag hint

Low Severity

In workaround mode, the missing-XML failure always appends a hint to add requires-test-xml-generation, even when that tag is already present. Tagged tests that still lack XML (for example on unsupported platforms where ensure_xml is omitted) get an incorrect remediation hint.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4ccd1df. Configure here.

if (missingBehavior == TestXmlMissingBehavior.FAIL
|| missingBehavior == TestXmlMissingBehavior.WORKAROUND) {
throw createTestExecException(TestAction.Code.MISSING_XML_OUTPUT, missingXmlMessage);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.MiddlemanAction;
import com.google.devtools.build.lib.actions.RunfilesTree;
import com.google.devtools.build.lib.analysis.AnalysisResult;
Expand Down Expand Up @@ -774,6 +775,7 @@ public void testEnsureXmlWrapsRunUnderAndTest() throws Exception {
"foo_test(",
" name = 'test',",
" srcs = ['test.sh'],",
" tags = ['requires-test-xml-generation'],",
")");
scratch.file("xml_wrapper/test.sh", "#!/bin/sh", "exit 0");
useConfiguration(
Expand All @@ -786,6 +788,8 @@ public void testEnsureXmlWrapsRunUnderAndTest() throws Exception {
Artifact ensureXml = testAction.getEnsureXmlExecutable();
assertThat(ensureXml).isNotNull();
assertThat(testAction.getInputs().toList()).contains(ensureXml);
assertThat(testAction.getExecutionInfo())
.containsKey(ExecutionRequirements.REQUIRES_TEST_XML_GENERATION);

ImmutableList<String> args =
TestStrategy.expandedArgsFromAction(testAction, /* ensureXml= */ true);
Expand All @@ -794,6 +798,21 @@ public void testEnsureXmlWrapsRunUnderAndTest() throws Exception {
assertThat(PathFragment.create(args.get(3)).getBaseName()).isEqualTo("test");
}

@Test
public void testEnsureXmlIsNotInputWithoutRequiresTestXmlGenerationTag() throws Exception {
useConfiguration("--experimental_test_xml_missing_behavior=workaround");

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 testEnsureXmlIsNotInputOutsideWorkaroundMode() throws Exception {
useConfiguration("--experimental_test_xml_missing_behavior=warn");
Expand Down Expand Up @@ -821,7 +840,11 @@ public void testEnsureXmlUsesTargetPlatformForTests() throws Exception {
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:aarch64',",
" ],",
")",
"foo_test(name = 'test', srcs = ['test.sh'])");
"foo_test(",
" name = 'test',",
" srcs = ['test.sh'],",
" tags = ['requires-test-xml-generation'],",
")");
scratch.file("xml_platform/test.sh", "#!/bin/sh", "exit 0");
useConfiguration(
"--experimental_test_xml_missing_behavior=workaround",
Expand Down Expand Up @@ -854,7 +877,11 @@ public void testEnsureXmlIsOmittedForUnsupportedExecutionPlatform() throws Excep
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:x86_64',",
" ],",
")",
"foo_test(name = 'test', srcs = ['test.sh'])");
"foo_test(",
" name = 'test',",
" srcs = ['test.sh'],",
" tags = ['requires-test-xml-generation'],",
")");
scratch.file("xml_platform/test.sh", "#!/bin/sh", "exit 0");
useConfiguration(
"--experimental_test_xml_missing_behavior=workaround",
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/exec/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
"//src/main/java/com/google/devtools/build/lib/actions:artifact_expander",
"//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/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink",
"//src/main/java/com/google/devtools/build/lib/actions:fileset_output_tree",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.DiscoveredModulesPruner;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.InputMetadataProvider;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnResult;
Expand Down Expand Up @@ -816,6 +817,15 @@ public void missingXmlStrictBehaviorFailsWithoutGeneratingXml(
() -> execute(testRunnerAction, actionExecutionContext, standaloneTestStrategy));

assertThat(exception).hasMessageThat().contains("failed to produce test.xml");
if (testXmlMissingBehavior == TestXmlMissingBehavior.WORKAROUND) {
assertThat(exception)
.hasMessageThat()
.contains(ExecutionRequirements.REQUIRES_TEST_XML_GENERATION);
} else {
assertThat(exception)
.hasMessageThat()
.doesNotContain(ExecutionRequirements.REQUIRES_TEST_XML_GENERATION);
}
verify(spawnStrategy, times(1)).exec(any(), any());
}

Expand All @@ -840,6 +850,7 @@ public void workaroundWrapperProducesXmlWithoutDerivedSpawn() throws Exception {
name = "wrapped_test",
size = "small",
srcs = ["wrapped_test.sh"],
tags = ["requires-test-xml-generation"],
)
""");
TestRunnerAction testRunnerAction = getTestAction("//standalone:wrapped_test");
Expand Down Expand Up @@ -892,6 +903,7 @@ public void workaroundWrapperIsDisabledWithoutSplitXmlGeneration() throws Except
name = "unwrapped_test",
size = "small",
srcs = ["unwrapped_test.sh"],
tags = ["requires-test-xml-generation"],
)
""");
TestRunnerAction testRunnerAction = getTestAction("//standalone:unwrapped_test");
Expand Down
Loading