From 45da6c7b9775abe83057add6f20162adeb5ca76c Mon Sep 17 00:00:00 2001 From: Kush Chakraborty Date: Tue, 31 Jan 2017 20:22:22 +0000 Subject: Throw an exception when persistent workers are used to run tests with use_testrunner=0 -- PiperOrigin-RevId: 146150454 MOS_MIGRATED_REVID=146150454 --- .../build/lib/rules/test/TestActionBuilder.java | 8 +++++- .../build/lib/rules/test/TestRunnerAction.java | 9 +++++- .../build/lib/worker/WorkerTestStrategy.java | 5 ++++ .../shell/bazel/persistent_test_runner_test.sh | 33 ++++++++++++++++++++-- 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java index b7becb4eaf..b84be6979a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.packages.TestSize; import com.google.devtools.build.lib.packages.TestTimeout; import com.google.devtools.build.lib.rules.test.TestProvider.TestParams; +import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; @@ -282,12 +283,17 @@ public final class TestActionBuilder { targetName.getRelative(shardRunDir + "coverage.micro.dat"), root); } + boolean useTestRunner = false; + if (ruleContext.attributes().has("use_testrunner", Type.BOOLEAN)) { + useTestRunner = ruleContext.attributes().get("use_testrunner", Type.BOOLEAN); + } env.registerAction(new TestRunnerAction( ruleContext.getActionOwner(), inputs, testRuntime, testLog, cacheStatus, coverageArtifact, microCoverageArtifact, testProperties, testEnv, executionSettings, - shard, run, config, ruleContext.getWorkspaceName())); + shard, run, config, ruleContext.getWorkspaceName(), + useTestRunner)); results.add(cacheStatus); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java index b84055110b..e9143087f4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java @@ -89,6 +89,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa private final int shardNum; private final int runNumber; private final String workspaceName; + private final boolean useTestRunner; // Mutable state related to test caching. private boolean checkedCaching = false; @@ -127,7 +128,8 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa int shardNum, int runNumber, BuildConfiguration configuration, - String workspaceName) { + String workspaceName, + boolean useTestRunner) { super(owner, inputs, // Note that this action only cares about the runfiles, not the mapping. new RunfilesSupplierImpl(new PathFragment("runfiles"), executionSettings.getRunfiles()), @@ -167,6 +169,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa this.undeclaredOutputsAnnotationsPath = undeclaredOutputsAnnotationsDir.getChild("ANNOTATIONS"); this.testInfrastructureFailure = baseDir.getChild("test.infrastructure_failure"); this.workspaceName = workspaceName; + this.useTestRunner = useTestRunner; Map mergedTestEnv = new HashMap<>(configuration.getTestEnv()); mergedTestEnv.putAll(extraTestEnv); @@ -585,6 +588,10 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa return executionSettings; } + public boolean useTestRunner() { + return useTestRunner; + } + public boolean isSharded() { return testShard != null; } diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java index de534209d3..030234d359 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.TestExecException; +import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.exec.StandaloneTestStrategy; import com.google.devtools.build.lib.rules.test.TestActionContext; @@ -83,6 +84,10 @@ public class WorkerTestStrategy extends StandaloneTestStrategy { Spawn spawn, ActionExecutionContext actionExecutionContext) throws ExecException, InterruptedException, IOException { + if (!action.useTestRunner()) { + throw new UserExecException("Tests that do not use the default test runner are incompatible" + + " with the persistent worker test strategy. Please use another test strategy"); + } List startupArgs = getStartUpArgs(action); return execInWorker( diff --git a/src/test/shell/bazel/persistent_test_runner_test.sh b/src/test/shell/bazel/persistent_test_runner_test.sh index 95980edb29..8af6d04a65 100755 --- a/src/test/shell/bazel/persistent_test_runner_test.sh +++ b/src/test/shell/bazel/persistent_test_runner_test.sh @@ -136,9 +136,36 @@ EOF || true } -# TODO(kush): Remove this fake test once we enable real tests -function test_placeholder_until_real_tests_are_enabled() { - echo "test_placeholder_until_real_tests_are_enabled" +function test_fail_without_testrunner() { + mkdir -p java/testrunners || fail "mkdir failed" + + cat > java/testrunners/TestWithoutRunner.java < java/testrunners/BUILD <& $TEST_log \ + || fail "Normal test execution should pass." + + bazel test --no_cache_test_results --test_strategy=experimental_worker >& $TEST_log \ + //java/testrunners:TestWithoutRunner \ + && fail "Test should have failed when running with an experimental runner." \ + || true + + expect_log \ + "Tests that do not use the default test runner are incompatible with the persistent worker" } run_suite "Persistent Test Runner tests" -- cgit v1.2.3