Roll forward of https://github.com/bazelbuild/bazel/commit/0f0a0d58725603cf2f1c175963360b525718a195 with fixes.
1. Add --incompatible_exclusive_test_sandboxed flag so users can enable this feature conditionally.
2. Users who want to run exclusive tests locally can add a 'local' tag to the test.
*** Original change description ***
Enable sandboxing and remote caching for the "exclusive" tag
There's no good reason why tests tagged with exclusive shouldn't
be able to use sandboxing and/or remote caching. The current
limitations have been kept from open sourcing Bazel and have
never been revisited since.
Remote execution will continue to be disabled because Bazel
can't control what else is running on a remote machine.
Closes #8983.
***
PiperOrigin-RevId: 337219101
diff --git a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html
index 7660791..8211580 100644
--- a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html
+++ b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html
@@ -86,8 +86,9 @@
<li><code>exclusive</code> will force the test to be run in the
"exclusive" mode, ensuring that no other tests are running at the
same time. Such tests will be executed in serial fashion after all build
- activity and non-exclusive tests have been completed. They will also always
- run locally and thus without sandboxing.
+ activity and non-exclusive tests have been completed. Remote execution is
+ disabled for such tests because Bazel doesn't have control over what's
+ running on a remote machine.
</li>
<li><code>manual</code> keyword will exclude the target from expansion of target pattern wildcards
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 f476e82..3f8577f 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
@@ -34,6 +34,7 @@
import com.google.devtools.common.options.OptionDefinition;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
+import com.google.devtools.common.options.OptionMetadataTag;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.TriState;
@@ -259,6 +260,20 @@
+ "coverage run.")
public boolean fetchAllCoverageOutputs;
+ @Option(
+ name = "incompatible_exclusive_test_sandboxed",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+ effectTags = {OptionEffectTag.UNKNOWN},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help =
+ "If true, exclusive tests will run with sandboxed strategy. Add 'local' tag to force "
+ + "an exclusive test run locally")
+ public boolean incompatibleExclusiveTestSandboxed;
+
@Override
public FragmentOptions getHost() {
TestOptions hostOptions = (TestOptions) getDefault();
@@ -370,6 +385,8 @@
return options.fetchAllCoverageOutputs;
}
+ public boolean incompatibleExclusiveTestSandboxed() { return options.incompatibleExclusiveTestSandboxed; }
+
/**
* Option converter that han handle two styles of value for "--runs_per_test":
*
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetProperties.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetProperties.java
index 0493dc6..84274ec 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetProperties.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetProperties.java
@@ -94,8 +94,24 @@
Map<String, String> executionInfo = Maps.newLinkedHashMap();
executionInfo.putAll(TargetUtils.getExecutionInfo(rule));
- if (TargetUtils.isLocalTestRule(rule) || TargetUtils.isExclusiveTestRule(rule)) {
- executionInfo.put(ExecutionRequirements.LOCAL, "");
+
+ boolean incompatibleExclusiveTestSandboxed = false;
+
+ TestConfiguration testConfiguration = ruleContext.getFragment(TestConfiguration.class);
+ if (testConfiguration != null) {
+ incompatibleExclusiveTestSandboxed = testConfiguration.incompatibleExclusiveTestSandboxed();
+ }
+
+ if (incompatibleExclusiveTestSandboxed) {
+ if (TargetUtils.isLocalTestRule(rule)) {
+ executionInfo.put(ExecutionRequirements.LOCAL, "");
+ } else if (TargetUtils.isExclusiveTestRule(rule)) {
+ executionInfo.put(ExecutionRequirements.NO_REMOTE_EXEC, "");
+ }
+ } else {
+ if (TargetUtils.isLocalTestRule(rule) || TargetUtils.isExclusiveTestRule(rule)) {
+ executionInfo.put(ExecutionRequirements.LOCAL, "");
+ }
}
if (executionRequirements != null) {
diff --git a/src/test/java/com/google/devtools/build/lib/rules/test/BUILD b/src/test/java/com/google/devtools/build/lib/rules/test/BUILD
index aae863e..cd2c9ac 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/test/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/rules/test/BUILD
@@ -16,6 +16,7 @@
name = "TestRulesTests_lib",
srcs = glob(["*.java"]),
deps = [
+ "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
"//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/test/TestTargetPropertiesTest.java b/src/test/java/com/google/devtools/build/lib/rules/test/TestTargetPropertiesTest.java
index 5571f5f..4be2f26 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/test/TestTargetPropertiesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/test/TestTargetPropertiesTest.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.test.TestProvider;
@@ -50,4 +51,63 @@
.getLocalResourceUsage(testAction.getOwner().getLabel(), false);
assertThat(localResourceUsage.getCpuUsage()).isEqualTo(4.0);
}
+
+ @Test
+ public void testTestWithExclusiveRunLocallyByDefault() throws Exception {
+ scratch.file("tests/test.sh", "#!/bin/bash", "exit 0");
+ scratch.file(
+ "tests/BUILD",
+ "sh_test(",
+ " name = 'test',",
+ " size = 'small',",
+ " srcs = ['test.sh'],",
+ " tags = ['exclusive'],",
+ ")");
+ ConfiguredTarget testTarget = getConfiguredTarget("//tests:test");
+ TestRunnerAction testAction =
+ (TestRunnerAction)
+ getGeneratingAction(TestProvider.getTestStatusArtifacts(testTarget).get(0));
+ assertThat(testAction.getExecutionInfo()).containsKey(ExecutionRequirements.LOCAL);
+ assertThat(testAction.getExecutionInfo()).hasSize(1);
+ }
+
+ @Test
+ public void testTestWithExclusiveDisablesRemoteExecution() throws Exception {
+ useConfiguration("--incompatible_exclusive_test_sandboxed");
+ scratch.file("tests/test.sh", "#!/bin/bash", "exit 0");
+ scratch.file(
+ "tests/BUILD",
+ "sh_test(",
+ " name = 'test',",
+ " size = 'small',",
+ " srcs = ['test.sh'],",
+ " tags = ['exclusive'],",
+ ")");
+ ConfiguredTarget testTarget = getConfiguredTarget("//tests:test");
+ TestRunnerAction testAction =
+ (TestRunnerAction)
+ getGeneratingAction(TestProvider.getTestStatusArtifacts(testTarget).get(0));
+ assertThat(testAction.getExecutionInfo()).containsKey(ExecutionRequirements.NO_REMOTE_EXEC);
+ assertThat(testAction.getExecutionInfo()).hasSize(1);
+ }
+
+ @Test
+ public void testTestWithExclusiveAndLocalRunLocally() throws Exception {
+ useConfiguration("--incompatible_exclusive_test_sandboxed");
+ scratch.file("tests/test.sh", "#!/bin/bash", "exit 0");
+ scratch.file(
+ "tests/BUILD",
+ "sh_test(",
+ " name = 'test',",
+ " size = 'small',",
+ " srcs = ['test.sh'],",
+ " tags = ['exclusive', 'local'],",
+ ")");
+ ConfiguredTarget testTarget = getConfiguredTarget("//tests:test");
+ TestRunnerAction testAction =
+ (TestRunnerAction)
+ getGeneratingAction(TestProvider.getTestStatusArtifacts(testTarget).get(0));
+ assertThat(testAction.getExecutionInfo()).containsKey(ExecutionRequirements.LOCAL);
+ assertThat(testAction.getExecutionInfo()).hasSize(1);
+ }
}
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index 03c5e68..26444e6 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -2141,6 +2141,36 @@
@local_foo//:all
}
+function test_exclusive_tag() {
+ # Test that the exclusive tag works with the remote cache.
+ mkdir -p a
+ cat > a/success.sh <<'EOF'
+#!/bin/sh
+exit 0
+EOF
+ chmod 755 a/success.sh
+ cat > a/BUILD <<'EOF'
+sh_test(
+ name = "success_test",
+ srcs = ["success.sh"],
+ tags = ["exclusive"],
+)
+EOF
+
+ bazel test \
+ --incompatible_exclusive_test_sandboxed \
+ --remote_cache=grpc://localhost:${worker_port} \
+ //a:success_test || fail "Failed to test //a:success_test"
+
+ bazel test \
+ --incompatible_exclusive_test_sandboxed \
+ --remote_cache=grpc://localhost:${worker_port} \
+ --nocache_test_results \
+ //a:success_test >& $TEST_log || fail "Failed to test //a:success_test"
+
+ expect_log "remote cache hit"
+}
+
# TODO(alpha): Add a test that fails remote execution when remote worker
# supports sandbox.
