Change the tag experimental_testrunner into a couple of flags.

Note 1) Explanation of the flags:
--explicit_java_test_deps: This is a flag independent of the others, and forces users to specify the JUnit deps. We should try to make this flag default to true in a future release, irrespective of the work around persistent java tests.

--experimental_testrunner: Bazel-only flag affecting switching from the BazelTestRunner to the ExperimentalTestRunner which will run the tests in a separate classloader. --explicit_java_test_deps is desired for this to ensure once we split the classpaths of the TestRunner and the TestTarget, the TestTarget does not hit a ClassNotFoundException, due to missing JUnit deps.

--test_strategy=experimental_worker: This is the existing flag, which in turn depends on --experimental_testrunner flag, since only the ExperimentalTestRunner is capable to running java tests persistently.

Note 2) There was no clean way to check for the flags defined in JavaOptions within TestActionBuilder (as I was checking the "tag=experimental_testrunner" before), without making TeasActionBuilder's build rules depend on java rules (yikes!). Hence, I created a new method compatibleWithStrategy() within each fragment, so I could check if WorkerTestStrategy could be compatible with the user specified flags. Thanks to Greg for suggesting this approach!

RELNOTES: None

PiperOrigin-RevId: 151729869

2 files changed, 28 insertions, 33 deletions

diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh
index 691511c041..8cbfc12ada 100755
--- a/src/test/shell/bazel/bazel_java_test.sh
+++ b/src/test/shell/bazel/bazel_java_test.sh
@@ -656,7 +656,7 @@ EOF
   expect_log "Test message"
 }
 
-function test_junit_deps_enforced_with_experimental_testrunner() {
+function test_explicit_java_test_deps_flag() {
   setup_javatest_support
   mkdir -p java/testrunners || fail "mkdir failed"
 
@@ -677,25 +677,25 @@ public class Tests {
 }
 EOF
 
-  # With experimental_testrunner, we fail without explicitly specifying the JUnit deps.
+  # With explicit_java_test_deps, we fail without explicitly specifying the JUnit deps.
   cat > java/testrunners/BUILD <<EOF
 java_test(name = "Tests",
           srcs = ['Tests.java'],
-          tags = ['experimental_testrunner'],
 )
 EOF
-  bazel test --test_output=streamed //java/testrunners:Tests &> "$TEST_log" && fail "Expected Failure" || true
+  bazel test --test_output=streamed --explicit_java_test_deps //java/testrunners:Tests \
+      &> "$TEST_log" && fail "Expected Failure" || true
   expect_log "cannot find symbol"
 
-  # We start passing again with experimental_testrunner once we explicitly specify the deps.
+  # We start passing again with explicit_java_test_deps once we explicitly specify the deps.
   cat > java/testrunners/BUILD <<EOF
 java_test(name = "Tests",
           srcs = ['Tests.java'],
-          tags = ['experimental_testrunner'],
           deps = ['//third_party:junit4'],
 )
 EOF
-  bazel test --test_output=streamed //java/testrunners:Tests &> "$TEST_log" || fail "Expected success"
+  bazel test --test_output=streamed --explicit_java_test_deps //java/testrunners:Tests \
+      &> "$TEST_log" || fail "Expected success"
   expect_log "testTest was run"
 }
 
diff --git a/src/test/shell/bazel/persistent_test_runner_test.sh b/src/test/shell/bazel/persistent_test_runner_test.sh
index bb01d497ef..76494f0b53 100755
--- a/src/test/shell/bazel/persistent_test_runner_test.sh
+++ b/src/test/shell/bazel/persistent_test_runner_test.sh
@@ -70,21 +70,20 @@ EOF
 java_test(name = "TestsPass",
           srcs = ['TestsPass.java'],
           deps = ['//third_party:junit4'],
-          tags = ["experimental_testrunner"],
 )
 
 java_test(name = "TestsFail",
           srcs = ['TestsFail.java'],
           deps = ['//third_party:junit4'],
-          tags = ["experimental_testrunner"],
 )
 EOF
 
-  bazel test --test_strategy=experimental_worker //java/testrunners:TestsPass \
-      || fail "Test fails unexpectedly"
+  bazel test --explicit_java_test_deps --experimental_testrunner --test_strategy=experimental_worker \
+      //java/testrunners:TestsPass || fail "Test fails unexpectedly"
 
-  bazel test --test_strategy=experimental_worker --test_output=all \
-      //java/testrunners:TestsFail &> $TEST_log && fail "Test passes unexpectedly" || true
+  bazel test --explicit_java_test_deps --experimental_testrunner --test_strategy=experimental_worker \
+      --test_output=all //java/testrunners:TestsFail &> $TEST_log \
+      && fail "Test passes unexpectedly" || true
   expect_log "Test is supposed to fail"
 }
 
@@ -113,13 +112,12 @@ EOF
   cat > java/testrunners/BUILD <<EOF
 java_test(name = "Tests",
           srcs = ['Tests.java'],
-          tags = ["experimental_testrunner"],
           deps = ['//third_party:junit4'],
 )
 EOF
 
-  bazel test --test_strategy=experimental_worker //java/testrunners:Tests &> $TEST_log \
-      || fail "Test fails unexpectedly"
+  bazel test --explicit_java_test_deps --experimental_testrunner --test_strategy=experimental_worker \
+      //java/testrunners:Tests &> $TEST_log || fail "Test fails unexpectedly"
 
   # Now get the test to fail.
   cat > java/testrunners/Tests.java <<EOF
@@ -140,8 +138,9 @@ public class Tests {
 }
 EOF
 
-  bazel test --test_strategy=experimental_worker --test_output=all --no_cache_test_results \
-      //java/testrunners:Tests &> $TEST_log && fail "Test passes unexpectedly" || true
+  bazel test --explicit_java_test_deps --experimental_testrunner --test_strategy=experimental_worker \
+      --test_output=all --no_cache_test_results //java/testrunners:Tests &> $TEST_log \
+      && fail "Test passes unexpectedly" || true
   expect_log "Test is supposed to fail now"
 }
 
@@ -171,13 +170,12 @@ EOF
   cat > java/testrunners/BUILD <<EOF
 java_test(name = "Tests",
           srcs = ['Tests.java'],
-          tags = ["experimental_testrunner"],
           deps = ['//third_party:junit4'],
 )
 EOF
 
-  bazel test --test_strategy=experimental_worker //java/testrunners:Tests &> $TEST_log \
-      || fail "Test fails unexpectedly"
+  bazel test --explicit_java_test_deps --experimental_testrunner --test_strategy=experimental_worker \
+      //java/testrunners:Tests &> $TEST_log || fail "Test fails unexpectedly"
 
   # Create a library to add a dep.
   cat > java/testrunners/TrueVal.java <<EOF
@@ -217,7 +215,6 @@ java_library(name = "trueval",
 
 java_test(name = "Tests",
           srcs = ['Tests.java'],
-          tags = ["experimental_testrunner"],
           deps = [
                    ':trueval',
                    '//third_party:junit4'
@@ -225,8 +222,9 @@ java_test(name = "Tests",
 )
 EOF
 
-  bazel test --test_strategy=experimental_worker --test_output=all --no_cache_test_results \
-      //java/testrunners:Tests &> $TEST_log  && fail "Test passes unexpectedly" || true
+  bazel test --explicit_java_test_deps --experimental_testrunner --test_strategy=experimental_worker \
+      --test_output=all --no_cache_test_results //java/testrunners:Tests &> $TEST_log \
+       && fail "Test passes unexpectedly" || true
   expect_log "Supposed to fail now."
 }
 
@@ -246,18 +244,16 @@ EOF
 java_test(name = "TestWithoutRunner",
           srcs = ['TestWithoutRunner.java'],
           use_testrunner = 0,
-          tags = ["experimental_testrunner"],
           main_class = "testrunners.TestWithoutRunner"
 )
 EOF
 
-  bazel test --no_cache_test_results //java/testrunners:TestWithoutRunner >& $TEST_log \
-      || fail "Normal test execution should pass."
+  bazel test --explicit_java_test_deps --experimental_testrunner --no_cache_test_results \
+      //java/testrunners:TestWithoutRunner >& $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
+  bazel test --explicit_java_test_deps --experimental_testrunner --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 experimental test runner are incompatible with the persistent worker"
@@ -299,8 +295,7 @@ EOF
       && fail "Test should have failed when running with an experimental runner." \
       || true
 
-  expect_log \
-      "Tests that do not use the experimental test runner are incompatible with the persistent worker"
+  expect_log "Build configuration not compatible with experimental_worker"
 }
 
 run_suite "Persistent Test Runner tests"