diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java index 9c4d5b118cdb5b..7b35bb839f0933 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java @@ -158,6 +158,14 @@ public TestActionBuilder setShardCount(int explicitShardCount) { return this; } + private boolean isPersistentTestRunner() { + return ruleContext + .getConfiguration() + .getFragment(TestConfiguration.class) + .isPersistentTestRunner() + && persistentTestRunnerRunfiles != null; + } + /** * Creates a test action and artifacts for the given rule. The test action will * use the specified executable and runfiles. @@ -188,8 +196,8 @@ private TestParams createTestAction(int shards) { PrerequisiteArtifacts.nestedSet(ruleContext, "$test_runtime", TransitionMode.HOST); inputsBuilder.addTransitive(testRuntime); } - TestTargetProperties testProperties = new TestTargetProperties( - ruleContext, executionRequirements); + TestTargetProperties testProperties = + new TestTargetProperties(ruleContext, executionRequirements, isPersistentTestRunner()); // If the test rule does not provide InstrumentedFilesProvider, there's not much that we can do. final boolean collectCodeCoverage = config.isCodeCoverageEnabled() @@ -301,7 +309,7 @@ private TestParams createTestAction(int shards) { } else { Artifact flagFile = null; // The worker spawn runner expects a flag file containg the worker's flags. - if (testConfiguration.isPersistentTestRunner()) { + if (isPersistentTestRunner()) { flagFile = ruleContext.getBinArtifact(ruleContext.getLabel().getName() + "_flag_file.txt"); inputsBuilder.add(flagFile); } @@ -366,7 +374,7 @@ private TestParams createTestAction(int shards) { testConfiguration.runsPerTestDetectsFlakes() && testConfiguration.cancelConcurrentTests(); RunfilesSupplier testRunfilesSupplier; - if (testConfiguration.isPersistentTestRunner()) { + if (isPersistentTestRunner()) { // Create a RunfilesSupplier from the persistent test runner's runfiles. Pass only the // test runner's runfiles to avoid using a different worker for every test run. testRunfilesSupplier = @@ -380,7 +388,7 @@ private TestParams createTestAction(int shards) { } ImmutableList.Builder tools = new ImmutableList.Builder<>(); - if (testConfiguration.isPersistentTestRunner()) { + if (isPersistentTestRunner()) { tools.add(testActionExecutable); tools.add(executionSettings.getExecutable()); tools.addAll(additionalTools.build()); 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 bf59452ceda06c..f476e822741012 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 @@ -327,6 +327,12 @@ public TestShardingStrategy testShardingStrategy() { return options.testShardingStrategy; } + /** + * Whether the persistent test runner is enabled. Note that not all test rules support this + * feature, in which case Bazel should fall back to the normal test runner. Therefore, this method + * must only be called by test rules, and never for test actions. For actions, use {@code + * TestTargetProperties.isPersistentTestRunner} instead. + */ public boolean isPersistentTestRunner() { return options.persistentTestRunner; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java index e79d8d3d203f31..2fb04dbae98deb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java @@ -584,7 +584,7 @@ public void setupEnvVariables(Map env, Duration timeout) { env.put("RUNFILES_MANIFEST_ONLY", "1"); } - if (testConfiguration.isPersistentTestRunner()) { + if (testProperties.isPersistentTestRunner()) { // Let the test runner know it runs persistently within a worker. env.put("PERSISTENT_TEST_RUNNER", "true"); } 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 15a422fd98e641..0493dc69b18e8b 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 @@ -70,13 +70,16 @@ private static ResourceSet getResourceSetFromSize(TestSize size) { private final boolean isExternal; private final String language; private final ImmutableMap executionInfo; + private final boolean isPersistentTestRunner; /** - * Creates test target properties instance. Constructor expects that it - * will be called only for test configured targets. + * Creates test target properties instance. Constructor expects that it will be called only for + * test configured targets. */ - TestTargetProperties(RuleContext ruleContext, - ExecutionInfo executionRequirements) { + TestTargetProperties( + RuleContext ruleContext, + ExecutionInfo executionRequirements, + boolean isPersistentTestRunner) { Rule rule = ruleContext.getRule(); Preconditions.checkState(TargetUtils.isTestRule(rule)); @@ -87,6 +90,7 @@ private static ResourceSet getResourceSetFromSize(TestSize size) { // We need to use method on ruleConfiguredTarget to perform validation. isFlaky = ruleContext.attributes().get("flaky", Type.BOOLEAN); isExternal = TargetUtils.isExternalTestRule(rule); + this.isPersistentTestRunner = isPersistentTestRunner; Map executionInfo = Maps.newLinkedHashMap(); executionInfo.putAll(TargetUtils.getExecutionInfo(rule)); @@ -133,6 +137,10 @@ public boolean isExternal() { return isExternal; } + public boolean isPersistentTestRunner() { + return isPersistentTestRunner; + } + public ResourceSet getLocalResourceUsage(Label label, boolean usingLocalTestJobs) throws UserExecException { if (usingLocalTestJobs) { diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index e0d24d14ad47cd..f8c7d1da532d6d 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -35,7 +35,6 @@ import com.google.devtools.build.lib.actions.TestExecException; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.test.TestAttempt; -import com.google.devtools.build.lib.analysis.test.TestConfiguration; import com.google.devtools.build.lib.analysis.test.TestResult; import com.google.devtools.build.lib.analysis.test.TestRunnerAction; import com.google.devtools.build.lib.analysis.test.TestRunnerAction.ResolvedPaths; @@ -117,7 +116,7 @@ public TestRunnerSpawn createTestRunnerSpawn( executionInfo.put(ExecutionRequirements.NO_CACHE, ""); } executionInfo.put(ExecutionRequirements.TIMEOUT, "" + getTimeout(action).getSeconds()); - if (action.getConfiguration().getFragment(TestConfiguration.class).isPersistentTestRunner()) { + if (action.getTestProperties().isPersistentTestRunner()) { executionInfo.put(ExecutionRequirements.SUPPORTS_WORKERS, "1"); } @@ -136,7 +135,7 @@ public TestRunnerSpawn createTestRunnerSpawn( action.getRunfilesSupplier(), ImmutableMap.of(), /*inputs=*/ action.getInputs(), - action.getConfiguration().getFragment(TestConfiguration.class).isPersistentTestRunner() + action.getTestProperties().isPersistentTestRunner() ? action.getTools() : NestedSetBuilder.emptySet(Order.STABLE_ORDER), ImmutableSet.copyOf(action.getSpawnOutputs()), diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index f8d0843ccda782..ca9ceb9df1088a 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -910,6 +910,18 @@ sh_test( ], ) +sh_test( + name = "persistent_test_runner_test", + size = "large", + srcs = ["persistent_test_runner_test.sh"], + data = [ + ":test-deps", + ], + tags = [ + "no_windows", + ], +) + sh_test( name = "bazel_workspace_status_test", size = "large", diff --git a/src/test/shell/bazel/persistent_test_runner_test.sh b/src/test/shell/bazel/persistent_test_runner_test.sh new file mode 100755 index 00000000000000..67f7028cb06336 --- /dev/null +++ b/src/test/shell/bazel/persistent_test_runner_test.sh @@ -0,0 +1,45 @@ +#!/bin/bash +# +# Copyright 2020 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Tests the persistent test runner +# + +# Load test environment +# Load the test setup defined in the parent directory +CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${CURRENT_DIR}/../integration_test_setup.sh" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + +cat >>$TEST_TMPDIR/bazelrc <<'EOF' +build --strategy=TestRunner=worker,local +build --experimental_persistent_test_runner +EOF + +function test_simple_sh_test() { + mkdir -p examples/tests/ + cat << 'EOF' > examples/tests/BUILD +sh_test( + name = "shell", + srcs = [ "shell.sh" ], +) +EOF + cat << 'EOF' > examples/tests/shell.sh +EOF + chmod +x examples/tests/shell.sh + bazel test examples/tests:shell &> $TEST_log || fail "Shell test failed" +} + +run_suite "persistent_test_runner"