From 960217631abdcab0a7ed95e2ab10acd55f636639 Mon Sep 17 00:00:00 2001 From: lberki Date: Mon, 5 Aug 2019 02:20:51 -0700 Subject: [PATCH] Automated rollback of commit 0f0a0d58725603cf2f1c175963360b525718a195. *** Reason for rollback *** Breaks some Google-internal targets. *** 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: 261644804 --- .../templates/attributes/common/tags.html | 5 ++-- .../analysis/test/TestTargetProperties.java | 5 +--- .../rules/test/TestTargetPropertiesTest.java | 20 ------------- .../bazel/remote/remote_execution_test.sh | 28 ------------------- 4 files changed, 3 insertions(+), 55 deletions(-) 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 2af4ae112c8f19..4011a0f6969fb6 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 @@ -88,9 +88,8 @@
  • exclusive keyword 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. Remote execution is - disabled for such tests because Bazel doesn't have control over what's - running on a remote machine. + activity and non-exclusive tests have been completed. They will also always + run locally and thus without sandboxing.
  • manual keyword will force the test target to not be included in target pattern 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 b5f51ceb0c7d85..e7a83540995e89 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 @@ -88,12 +88,9 @@ private static ResourceSet getResourceSetFromSize(TestSize size) { Map executionInfo = Maps.newLinkedHashMap(); executionInfo.putAll(TargetUtils.getExecutionInfo(rule)); - if (TargetUtils.isLocalTestRule(rule)) { + if (TargetUtils.isLocalTestRule(rule) || TargetUtils.isExclusiveTestRule(rule)) { executionInfo.put(ExecutionRequirements.LOCAL, ""); } - if (TargetUtils.isExclusiveTestRule(rule)) { - executionInfo.put(ExecutionRequirements.NO_REMOTE_EXEC, ""); - } if (executionRequirements != null) { // This will overwrite whatever TargetUtils put there, which might be confusing. 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 bdb86e1c90a8f1..5571f5fcde4ec7 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,7 +16,6 @@ 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; @@ -51,23 +50,4 @@ public void testTestWithCpusTagHasCorrectLocalResourcesEstimate() throws Excepti .getLocalResourceUsage(testAction.getOwner().getLabel(), false); assertThat(localResourceUsage.getCpuUsage()).isEqualTo(4.0); } - - @Test - public void testTestWithExclusiveDisablesRemoteExecution() 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.NO_REMOTE_EXEC); - 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 a70ce9f7d31e81..328bae19efa1c6 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -1131,34 +1131,6 @@ EOF expect_log "uri:.*bytestream://localhost" } -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 \ - --remote_cache=grpc://localhost:${worker_port} \ - //a:success_test || fail "Failed to test //a:success_test" - - bazel test \ - --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.