Skip to content

Commit

Permalink
Automated rollback of commit 0f0a0d5.
Browse files Browse the repository at this point in the history
*** 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 bazelbuild#8983.

PiperOrigin-RevId: 261644804
  • Loading branch information
lberki authored and Copybara-Service committed Aug 5, 2019
1 parent bd4ca0b commit 9602176
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 55 deletions.
Expand Up @@ -88,9 +88,8 @@
<li><code>exclusive</code> keyword will force the test to be run in the
&quot;exclusive&quot; 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.
</li>

<li><code>manual</code> keyword will force the test target to not be included in target pattern
Expand Down
Expand Up @@ -88,12 +88,9 @@ private static ResourceSet getResourceSetFromSize(TestSize size) {

Map<String, String> 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.
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
28 changes: 0 additions & 28 deletions src/test/shell/bazel/remote/remote_execution_test.sh
Expand Up @@ -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.

Expand Down

0 comments on commit 9602176

Please sign in to comment.