Separate the classpaths of the TestRunner with the test target, and use a separate Classloader to load the test target's classes. This enables a clean separation of the classes of the TestRunner with the target under test.

This is achieved with the following steps:
1. Start the test runner with only the bare bones classpaths to the Test Runner's classes which are used by the system ClassLoader.
2. Have all the classpaths required to load the test target's classes in a TEST_TARGET_CLASSPATH environment variable exported by the stub script.
3. Use a new classloader to load all the test target's classes using the paths in TEST_TARGET_CLASSPATH.

This additionally enables the persistent test runner (currently experimental), to reload all the target's classes for every subsequent test run, so it can pick up any changes to the classes in between runs.

The persistent test runner can be used by adding the argument --test_strategy=experimental_worker to the bazel test command.
Tested this against:
1. gerrit/gerrit-common:client_tests: Dismal avg. improvement of 580ms to 557ms  (just 23ms)
2. intellij/intellij/base:unit_tests: Somewhat modest avg. improvement 1661ms to 913ms (748 ms)

RELNOTES: 1) Java tests and suites will now have to explicitly declare JUnit dependency 2) All non-legacy java_tests will now be run in a

--
PiperOrigin-RevId: 148309979
MOS_MIGRATED_REVID=148309979

14 files changed, 319 insertions, 113 deletions

diff --git a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
index 1699cb882b..71a715a85a 100644
--- a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
+++ b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BUILD
@@ -17,6 +17,7 @@ java_library(
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/model",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/sharding",
         "//src/java_tools/junitrunner/java/com/google/testing/junit/runner/util",
+        "//src/main/protobuf:worker_protocol_java_proto",
         "//third_party:junit4",
     ],
 )
diff --git a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
index 74a6826f88..3f4c966d94 100644
--- a/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
+++ b/src/java_tools/junitrunner/java/com/google/testing/junit/runner/BazelTestRunner.java
@@ -14,13 +14,20 @@
 
 package com.google.testing.junit.runner;
 
+import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
+import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse;
 import com.google.testing.junit.runner.internal.StackTraces;
 import com.google.testing.junit.runner.junit4.JUnit4InstanceModules.Config;
 import com.google.testing.junit.runner.junit4.JUnit4InstanceModules.SuiteClass;
 import com.google.testing.junit.runner.junit4.JUnit4Runner;
 import com.google.testing.junit.runner.model.AntXmlResultWriter;
 import com.google.testing.junit.runner.model.XmlResultWriter;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
 import java.io.PrintStream;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
 import java.util.Date;
@@ -48,6 +55,9 @@ public class BazelTestRunner {
    */
   static final String TEST_SUITE_PROPERTY_NAME = "bazel.test_suite";
 
+  private static URL[] classpaths = null;
+  private static URLClassLoader targetClassLoader;
+
   private BazelTestRunner() {
     // utility class; should not be instantiated
   }
@@ -72,9 +82,8 @@ public class BazelTestRunner {
 
     String suiteClassName = System.getProperty(TEST_SUITE_PROPERTY_NAME);
 
-    if (args.length >= 1 && args[args.length - 1].equals("--persistent_test_runner")) {
-      System.err.println("Requested test strategy is currently unsupported.");
-      System.exit(1);
+    if ("true".equals(System.getenv("PERSISTENT_TEST_RUNNER"))) {
+      System.exit(runPersistentTestRunner(suiteClassName));
     }
 
     if (!checkTestSuiteProperty(suiteClassName)) {
@@ -126,7 +135,7 @@ public class BazelTestRunner {
   }
 
   private static int runTestsInSuite(String suiteClassName, String[] args) {
-    Class<?> suite = getTestClass(suiteClassName);
+    Class<?> suite = getTargetSuiteClass(suiteClassName);
 
     if (suite == null) {
       // No class found corresponding to the system property passed in from Bazel
@@ -136,29 +145,123 @@ public class BazelTestRunner {
       }
     }
 
-    // TODO(kush): Use a new classloader for the following instantiation.
     JUnit4Runner runner =
         JUnit4Bazel.builder()
             .suiteClass(new SuiteClass(suite))
             .config(new Config(args))
             .build()
             .runner();
-    return runner.run().wasSuccessful() ? 0 : 1;
+
+    // Some frameworks such as Mockito use the Thread's context classloader.
+    Thread.currentThread().setContextClassLoader(targetClassLoader);
+
+    int result = 1;
+    try {
+      result = runner.run().wasSuccessful() ? 0 : 1;
+    } catch (RuntimeException e) {
+      System.err.println("Test run failed with exception");
+      e.printStackTrace();
+    }
+    return result;
   }
 
-  private static Class<?> getTestClass(String name) {
-    if (name == null) {
+  /**
+   * Run in a loop awaiting instructions for the next test run.
+   *
+   * @param suiteClassName name of the class which is passed on to JUnit to determine the test suite
+   * @return 0 when we encounter an EOF from input, or non-zero values if we encounter an
+   *     unrecoverable error.
+   */
+  private static int runPersistentTestRunner(String suiteClassName) {
+    PrintStream originalStdOut = System.out;
+    PrintStream originalStdErr = System.err;
+
+    while (true) {
+      try {
+        WorkRequest request = WorkRequest.parseDelimitedFrom(System.in);
+
+        if (request == null) {
+          break;
+        }
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+        PrintStream printStream = new PrintStream(outputStream, true);
+        System.setOut(printStream);
+        System.setErr(printStream);
+        String[] arguments = request.getArgumentsList().toArray(new String[0]);
+        int exitCode = -1;
+        try {
+          exitCode = runTestsInSuite(suiteClassName, arguments);
+        } finally {
+          System.setOut(originalStdOut);
+          System.setErr(originalStdErr);
+        }
+
+        WorkResponse response =
+            WorkResponse
+                .newBuilder()
+                .setOutput(outputStream.toString())
+                .setExitCode(exitCode)
+                .build();
+        response.writeDelimitedTo(System.out);
+        System.out.flush();
+
+      } catch (IOException e) {
+        e.printStackTrace();
+        return 1;
+      }
+    }
+    return 0;
+  }
+
+  /**
+   * Get the actual Test Suite class corresponding to the given name.
+   */
+  private static Class<?> getTargetSuiteClass(String suiteClassName) {
+    if (suiteClassName == null) {
       return null;
     }
 
     try {
-      return Class.forName(name);
-    } catch (ClassNotFoundException e) {
+      targetClassLoader = new URLClassLoader(getClasspaths());
+      Class<?> targetSuiteClass = targetClassLoader.loadClass(suiteClassName);
+      System.out.printf(
+          "Running test suites for class: %s, created by classLoader: %s%n",
+          targetSuiteClass, targetSuiteClass.getClassLoader());
+      return targetSuiteClass;
+    } catch (ClassNotFoundException | MalformedURLException e) {
+      System.err.println("Exception in loading class:" + e.getMessage());
       return null;
     }
   }
 
   /**
+   * Used to get the classpaths which should be used to load the classes of the test target.
+   *
+   * @throws MalformedURLException when we are unable to create a given classpath.
+   * @return array of URLs containing the classpaths or null if classpaths could not be located.
+   */
+  private static URL[] getClasspaths() throws MalformedURLException {
+    // TODO(kush): WARNING THIS DOES NOT RELOAD CLASSPATHS FOR EVERY TEST RUN. b/34712039
+    if (classpaths != null) {
+      return classpaths;
+    }
+    String testTargetsClaspaths = System.getenv("TEST_TARGET_CLASSPATH");
+    if (testTargetsClaspaths == null || testTargetsClaspaths.isEmpty()) {
+      throw new IllegalStateException(
+          "Target's classpath not present in TEST_TARGET_CLASSPATH environment variable");
+    }
+
+    String[] targetClassPaths = testTargetsClaspaths.split(":");
+
+    classpaths = new URL[targetClassPaths.length];
+    String workingDir = System.getProperty("user.dir");
+    for (int index = 0; index < targetClassPaths.length; index++) {
+      classpaths[index] = new URL("file://" + workingDir + "/" + targetClassPaths[index]);
+    }
+    return classpaths;
+  }
+
+  /**
    * Prints out stack traces if the JVM does not exit quickly. This can help detect shutdown hooks
    * that are preventing the JVM from exiting quickly.
    *
diff --git a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/BUILD b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/BUILD
index 95327df8ab..3ba2806cf2 100644
--- a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/BUILD
+++ b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/BUILD
@@ -68,8 +68,12 @@ sh_test(
     args = [
         "$(location :TestbedBinary)",
         "bazel.test_suite",
+        "$(location //src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar)",
+    ],
+    data = [
+        ":TestbedBinary",
+        "//src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar",
     ],
-    data = [":TestbedBinary"],
     shard_count = 0,
     deps = [":testenv"],
 )
@@ -81,8 +85,12 @@ sh_test(
     args = [
         "$(location :TestbedBinary)",
         "bazel.test_suite",
+        "$(location //src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar)",
+    ],
+    data = [
+        ":TestbedBinary",
+        "//src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar",
     ],
-    data = [":TestbedBinary"],
     shard_count = 0,
     deps = [":testenv"],
 )
@@ -95,10 +103,12 @@ sh_test(
         "$(location :TestbedBinary)",
         "$(location //src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:XmlOutputExercises.ant.xml)",
         "bazel.test_suite",
+        "$(location //src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar)",
     ],
     data = [
         ":TestbedBinary",
         "//src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:XmlOutputExercises.ant.xml",
+        "//src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/testbed:libtestbed.jar",
     ],
     shard_count = 0,
     deps = [":testenv"],
diff --git a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/antxmlresultwriter_integration_test.sh b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/antxmlresultwriter_integration_test.sh
index 609518a6bf..b79ccf14fe 100755
--- a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/antxmlresultwriter_integration_test.sh
+++ b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/antxmlresultwriter_integration_test.sh
@@ -25,6 +25,7 @@ XML_OUTPUT_FILE="${TEST_TMPDIR}/test.xml"
 SUITE_PARAMETER="$3"
 SUITE="com.google.testing.junit.runner.testbed.XmlOutputExercises"
 SUITE_FLAG="-D${SUITE_PARAMETER}=${SUITE}"
+TESTBED_LIB_LOCATION="${PWD}/$4"
 
 shift 3
 source ${DIR}/testenv.sh || { echo "testenv.sh not found!" >&2; exit 1; }
@@ -32,6 +33,9 @@ source ${DIR}/testenv.sh || { echo "testenv.sh not found!" >&2; exit 1; }
 function test_XmlOutputExercises() {
   cd $TEST_TMPDIR
 
+  # Pass the test target classpath through the environment variable
+  declare -x TEST_TARGET_CLASSPATH="$TESTBED_LIB_LOCATION"
+
   $TESTBED --jvm_flag=${SUITE_FLAG} || true  # Test failures
 
   # Remove timestamps and test runtime from the XML files as they will always differ and cause a
diff --git a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/junit4_testbridge_integration_tests.sh b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/junit4_testbridge_integration_tests.sh
index b44c34783b..470fece506 100755
--- a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/junit4_testbridge_integration_tests.sh
+++ b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/junit4_testbridge_integration_tests.sh
@@ -25,6 +25,7 @@ DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 
 TESTBED="${PWD}/$1"
 SUITE_PARAMETER="$2"
+TESTBED_LIB_LOCATION="${PWD}/$3"
 SUITE_FLAG="-D${SUITE_PARAMETER}=com.google.testing.junit.runner.testbed.JUnit4TestbridgeExercises"
 XML_OUTPUT_FILE="${TEST_TMPDIR}/test.xml"
 
@@ -42,6 +43,9 @@ function set_up() {
 function test_Junit4() {
   cd $TEST_TMPDIR
 
+  # Pass the test target classpath through the environment variable
+  declare -x TEST_TARGET_CLASSPATH="$TESTBED_LIB_LOCATION"
+
   # Run the test without environment flag; it should fail.
   declare +x TESTBRIDGE_TEST_ONLY
   $TESTBED --jvm_flag=${SUITE_FLAG} >& $TEST_log && fail "Expected failure"
@@ -66,6 +70,9 @@ function test_Junit4() {
 function test_Junit4FlagOverridesEnv() {
   cd $TEST_TMPDIR
 
+  # Pass the test target classpath through the environment variable
+  declare -x TEST_TARGET_CLASSPATH="$TESTBED_LIB_LOCATION"
+
   # Run the test with both environment and command line flags.
   declare -x TESTBRIDGE_TEST_ONLY="doNotRun"
   $TESTBED --jvm_flag=${SUITE_FLAG} --test_filter doRun >& $TEST_log || \
diff --git a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/utf8_test_log_test.sh b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/utf8_test_log_test.sh
index 3b2c47c7bb..96dfaee784 100755
--- a/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/utf8_test_log_test.sh
+++ b/src/java_tools/junitrunner/javatests/com/google/testing/junit/runner/utf8_test_log_test.sh
@@ -23,6 +23,7 @@ TESTBED="${PWD}/$1"
 SUITE_PARAMETER="$2"
 SUITE_FLAG="-D${SUITE_PARAMETER}=com.google.testing.junit.runner.testbed.InternationalCharsTest"
 XML_OUTPUT_FILE="${TEST_TMPDIR}/test.xml"
+TESTBED_LIB_LOCATION="${PWD}/$3"
 unset TEST_PREMATURE_EXIT_FILE
 
 shift 2
@@ -35,6 +36,9 @@ function expect_log() {
 }
 
 function test_utf8_log() {
+  # Pass the test target classpath through the environment variable
+  declare -x TEST_TARGET_CLASSPATH="$TESTBED_LIB_LOCATION"
+
   $TESTBED --jvm_flag=${SUITE_FLAG} > $TEST_log && fail "Expected failure"
   expect_log 'expected:<Test [Japan].> but was:<Test [日本].>'
 
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
index f5fa1ff7ae..d988eb31ae 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
@@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
-import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
@@ -33,18 +32,22 @@ import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Su
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Template;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.bazel.rules.BazelConfiguration;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder;
 import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression;
 import com.google.devtools.build.lib.rules.java.JavaCommon;
+import com.google.devtools.build.lib.rules.java.JavaCompilationArgs;
+import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
 import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts;
 import com.google.devtools.build.lib.rules.java.JavaCompilationHelper;
 import com.google.devtools.build.lib.rules.java.JavaConfiguration;
 import com.google.devtools.build.lib.rules.java.JavaHelper;
 import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider;
-import com.google.devtools.build.lib.rules.java.JavaRunfilesProvider;
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
 import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
 import com.google.devtools.build.lib.rules.java.JavaTargetAttributes;
@@ -81,6 +84,8 @@ public class BazelJavaSemantics implements JavaSemantics {
   private static final String JACOCO_COVERAGE_RUNNER_MAIN_CLASS =
       "com.google.testing.coverage.JacocoCoverageRunner";
 
+  private static final String BAZEL_TEST_RUNNER = "com.google.testing.junit.runner.BazelTestRunner";
+
   private BazelJavaSemantics() {
   }
 
@@ -103,10 +108,20 @@ public class BazelJavaSemantics implements JavaSemantics {
 
   private static final String JUNIT4_RUNNER = "org.junit.runner.JUnitCore";
 
+  @Nullable
   private String getMainClassInternal(RuleContext ruleContext, ImmutableList<Artifact> sources) {
     if (!ruleContext.attributes().get("create_executable", Type.BOOLEAN)) {
       return null;
     }
+    String mainClass = determineMainClassFromRule(ruleContext);
+
+    if (mainClass.isEmpty()) {
+      mainClass = JavaCommon.determinePrimaryClass(ruleContext, sources);
+    }
+    return mainClass;
+  }
+
+  private String determineMainClassFromRule(RuleContext ruleContext) {
     String mainClass = ruleContext.attributes().get("main_class", Type.STRING);
 
     // Legacy behavior for java_test rules: main_class defaulted to JUnit4 runner.
@@ -120,9 +135,8 @@ public class BazelJavaSemantics implements JavaSemantics {
     if (mainClass.isEmpty()) {
       if (ruleContext.attributes().get("use_testrunner", Type.BOOLEAN)
           && !useLegacyJavaTest(ruleContext)) {
-        return "com.google.testing.junit.runner.BazelTestRunner";
+        return BAZEL_TEST_RUNNER;
       }
-      mainClass = JavaCommon.determinePrimaryClass(ruleContext, sources);
     }
     return mainClass;
   }
@@ -164,10 +178,55 @@ public class BazelJavaSemantics implements JavaSemantics {
     return ruleContext.getPrerequisiteArtifacts("resources", Mode.TARGET).list();
   }
 
+  /**
+   * Used to generate the Classpaths contained within the stub.
+   */
+  private static class ComputedClasspathSubstitution extends ComputedSubstitution {
+    private final NestedSet<Artifact> jars;
+    private final String workspacePrefix;
+    private final boolean isRunfilesEnabled;
+
+    ComputedClasspathSubstitution(
+        String key, NestedSet<Artifact> jars, String workspacePrefix, boolean isRunfilesEnabled) {
+      super(key);
+      this.jars = jars;
+      this.workspacePrefix = workspacePrefix;
+      this.isRunfilesEnabled = isRunfilesEnabled;
+    }
+
+    /**
+     * Builds a class path by concatenating the root relative paths of the artifacts. Each relative
+     * path entry is prepended with "${RUNPATH}" which will be expanded by the stub script at
+     * runtime, to either "${JAVA_RUNFILES}/" or if we are lucky, the empty string.
+     */
+    @Override
+    public String getValue() {
+      StringBuilder buffer = new StringBuilder();
+      buffer.append("\"");
+      for (Artifact artifact : jars) {
+        if (buffer.length() > 1) {
+          buffer.append(File.pathSeparatorChar);
+        }
+        if (!isRunfilesEnabled) {
+          buffer.append("$(rlocation ");
+          PathFragment runfilePath =
+              new PathFragment(new PathFragment(workspacePrefix), artifact.getRunfilesPath());
+          buffer.append(runfilePath.normalize().getPathString());
+          buffer.append(")");
+        } else {
+          buffer.append("${RUNPATH}");
+          buffer.append(artifact.getRunfilesPath().getPathString());
+        }
+      }
+      buffer.append("\"");
+      return buffer.toString();
+    }
+  }
+
   @Override
   public Artifact createStubAction(
       RuleContext ruleContext,
-      final JavaCommon javaCommon,
+      JavaCommon javaCommon,
       List<String> jvmFlags,
       Artifact executable,
       String javaStartClass,
@@ -188,18 +247,26 @@ public class BazelJavaSemantics implements JavaSemantics {
     arguments.add(Substitution.of("%javabin%", javaExecutable));
     arguments.add(Substitution.of("%needs_runfiles%",
         ruleContext.getFragment(Jvm.class).getJavaExecutable().isAbsolute() ? "0" : "1"));
+
+    NestedSet<Artifact> classpath = javaCommon.getRuntimeClasspath();
+    NestedSet<Artifact> testTargetClasspath = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+    TransitiveInfoCollection testSupport = getTestSupport(ruleContext);
+    if (TargetUtils.isTestRule(ruleContext.getRule())
+        && determineMainClassFromRule(ruleContext).equals(BAZEL_TEST_RUNNER)
+        && testSupport != null) {
+      // Keep only the locations containing the classes to start the test runner itself within,
+      // classpath variable, and place all the paths required for the test run in
+      // testTargetClasspath, so that the classes for the test target may be loaded by a separate
+      // ClassLoader.
+      testTargetClasspath = classpath;
+      classpath = getRuntimeJarsForTargets(testSupport);
+    }
     arguments.add(
-        new ComputedSubstitution("%classpath%") {
-          @Override
-          public String getValue() {
-            StringBuilder buffer = new StringBuilder();
-            Iterable<Artifact> jars = javaCommon.getRuntimeClasspath();
-            char delimiter = File.pathSeparatorChar;
-            appendRunfilesRelativeEntries(
-                buffer, jars, workspacePrefix, delimiter, isRunfilesEnabled);
-            return buffer.toString();
-          }
-        });
+        new ComputedClasspathSubstitution(
+            "%classpath%", classpath, workspacePrefix, isRunfilesEnabled));
+    arguments.add(
+        new ComputedClasspathSubstitution(
+            "%test_target_classpath%", testTargetClasspath, workspacePrefix, isRunfilesEnabled));
 
     JavaCompilationArtifacts javaArtifacts = javaCommon.getJavaCompilationArtifacts();
     String path =
@@ -250,40 +317,7 @@ public class BazelJavaSemantics implements JavaSemantics {
     }
   }
 
-  /**
-   * Builds a class path by concatenating the root relative paths of the artifacts separated by the
-   * delimiter. Each relative path entry is prepended with "${RUNPATH}" which will be expanded by
-   * the stub script at runtime, to either "${JAVA_RUNFILES}/" or if we are lucky, the empty string.
-   *
-   * @param buffer the buffer to use for concatenating the entries
-   * @param artifacts the entries to concatenate in the buffer
-   * @param delimiter the delimiter character to separate the entries
-   */
-  private static void appendRunfilesRelativeEntries(
-      StringBuilder buffer,
-      Iterable<Artifact> artifacts,
-      String workspacePrefix,
-      char delimiter,
-      boolean isRunfilesEnabled) {
-    buffer.append("\"");
-    for (Artifact artifact : artifacts) {
-      if (buffer.length() > 1) {
-        buffer.append(delimiter);
-      }
-      if (!isRunfilesEnabled) {
-        buffer.append("$(rlocation ");
-        PathFragment runfilePath =
-            new PathFragment(new PathFragment(workspacePrefix), artifact.getRunfilesPath());
-        buffer.append(runfilePath.normalize().getPathString());
-        buffer.append(")");
-      } else {
-        buffer.append("${RUNPATH}");
-        buffer.append(artifact.getRunfilesPath().getPathString());
-      }
-    }
-    buffer.append("\"");
-  }
-
+  @Nullable
   private TransitiveInfoCollection getTestSupport(RuleContext ruleContext) {
     if (!isJavaBinaryOrJavaTest(ruleContext)) {
       return null;
@@ -300,13 +334,25 @@ public class BazelJavaSemantics implements JavaSemantics {
     }
   }
 
+  private static NestedSet<Artifact> getRuntimeJarsForTargets(TransitiveInfoCollection... deps) {
+    // The dep may be a simple JAR and not a java rule, hence we can't simply do
+    // dep.getProvider(JavaCompilationArgsProvider.class).getRecursiveJavaCompilationArgs(),
+    // so we reuse the logic within JavaCompilationArgs to handle both scenarios.
+    JavaCompilationArgs args =
+        JavaCompilationArgs.builder()
+            .addTransitiveTargets(
+                ImmutableList.copyOf(deps), /*recursive=*/ true, ClasspathType.RUNTIME_ONLY)
+            .build();
+    return args.getRuntimeJars();
+  }
+
   @Override
   public void addRunfilesForBinary(RuleContext ruleContext, Artifact launcher,
       Runfiles.Builder runfilesBuilder) {
     TransitiveInfoCollection testSupport = getTestSupport(ruleContext);
     if (testSupport != null) {
-      runfilesBuilder.addTarget(testSupport, JavaRunfilesProvider.TO_RUNFILES);
-      runfilesBuilder.addTarget(testSupport, RunfilesProvider.DEFAULT_RUNFILES);
+      // Not using addTransitiveArtifacts() due to the mismatch in NestedSet ordering.
+      runfilesBuilder.addArtifacts(getRuntimeJarsForTargets(testSupport));
     }
   }
 
@@ -316,12 +362,16 @@ public class BazelJavaSemantics implements JavaSemantics {
 
   @Override
   public void collectTargetsTreatedAsDeps(
-      RuleContext ruleContext, ImmutableList.Builder<TransitiveInfoCollection> builder) {
+      RuleContext ruleContext,
+      ImmutableList.Builder<TransitiveInfoCollection> builder,
+      ClasspathType type) {
+    if (type == ClasspathType.COMPILE_ONLY) {
+      // We add the test support below, but the test framework's deps are not relevant for
+      // COMPILE_ONLY, hence we return here.
+      return;
+    }
     TransitiveInfoCollection testSupport = getTestSupport(ruleContext);
     if (testSupport != null) {
-      // TODO(bazel-team): The testsupport is used as the test framework
-      // and really only needs to be on the runtime, not compile-time
-      // classpath.
       builder.add(testSupport);
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt
index cdc7ee4aa7..312976f586 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt
@@ -228,6 +228,10 @@ else
   CLASSPATH=%classpath%
 fi
 
+# Export the classpaths which would be used to load the classes of the test target as an
+# environment variable.
+export TEST_TARGET_CLASSPATH=%test_target_classpath%
+
 # If using Jacoco in offline instrumentation mode, the CLASSPATH contains instrumented files.
 # We need to make the metadata jar with uninstrumented classes available for generating
 # the lcov-compatible coverage report, and we don't want it on the classpath.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
index 03c6a594a3..8056e7cb0d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
@@ -95,10 +95,6 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
       }
     }
 
-    List<TransitiveInfoCollection> deps =
-        // Do not remove <TransitiveInfoCollection>: workaround for Java 7 type inference.
-        Lists.<TransitiveInfoCollection>newArrayList(
-            common.targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY));
     semantics.checkRule(ruleContext, common);
     semantics.checkForProtoLibraryAndJavaProtoLibraryOnSameProto(ruleContext, common);
     String mainClass = semantics.getMainClass(ruleContext, common.getSrcsArtifacts());
@@ -110,6 +106,10 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
     // Collect the transitive dependencies.
     JavaCompilationHelper helper = new JavaCompilationHelper(
         ruleContext, semantics, common.getJavacOpts(), attributesBuilder);
+    List<TransitiveInfoCollection> deps =
+        // Do not remove <TransitiveInfoCollection>: workaround for Java 7 type inference.
+        Lists.<TransitiveInfoCollection>newArrayList(
+            common.targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY));
     helper.addLibrariesToAttributes(deps);
     attributesBuilder.addNativeLibraries(
         collectNativeLibraries(common.targetsTreatedAsDeps(ClasspathType.BOTH)));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
index 344793423e..b6f0f2b448 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java
@@ -664,7 +664,7 @@ public class JavaCommon {
     }
     builder.addAll(ruleContext.getPrerequisites("deps", Mode.TARGET));
 
-    semantics.collectTargetsTreatedAsDeps(ruleContext, builder);
+    semantics.collectTargetsTreatedAsDeps(ruleContext, builder, type);
 
     // Implicitly add dependency on java launcher cc_binary when --java_launcher= is enabled,
     // or when launcher attribute is specified in a build rule.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
index ed638b2147..9f5034c1dc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
@@ -37,6 +37,7 @@ import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression;
+import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType;
 import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
 import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -262,7 +263,7 @@ public interface JavaSemantics {
    */
   Artifact createStubAction(
       RuleContext ruleContext,
-      final JavaCommon javaCommon,
+      JavaCommon javaCommon,
       List<String> jvmFlags,
       Artifact executable,
       String javaStartClass,
@@ -284,11 +285,11 @@ public interface JavaSemantics {
    */
   Iterable<String> getExtraJavacOpts(RuleContext ruleContext);
 
-  /**
-   * Add additional targets to be treated as direct dependencies.
-   */
+  /** Add additional targets to be treated as direct dependencies. */
   void collectTargetsTreatedAsDeps(
-      RuleContext ruleContext, ImmutableList.Builder<TransitiveInfoCollection> builder);
+      RuleContext ruleContext,
+      ImmutableList.Builder<TransitiveInfoCollection> builder,
+      ClasspathType type);
 
   /**
    * Enables coverage support for the java target - adds instrumented jar to the classpath and
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 d4a0fa8140..e4193e8420 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
@@ -92,7 +92,7 @@ public class WorkerTestStrategy extends StandaloneTestStrategy {
     return execInWorker(
         action,
         actionExecutionContext,
-        spawn.getEnvironment(),
+        addPersistentRunnerVars(spawn.getEnvironment()),
         startupArgs,
         actionExecutionContext.getExecutor().getExecRoot(),
         maxRetries);
@@ -107,6 +107,12 @@ public class WorkerTestStrategy extends StandaloneTestStrategy {
       int retriesLeft)
       throws ExecException, InterruptedException, IOException {
     Executor executor = actionExecutionContext.getExecutor();
+    // TODO(kush): Remove once we're out of the experimental phase.
+    executor
+        .getEventHandler()
+        .handle(
+            Event.warn(
+                "RUNNING TEST IN AN EXPERIMENTAL PERSISTENT WORKER. RESULTS MAY BE INACCURATE"));
     TestResultData.Builder builder = TestResultData.newBuilder();
 
     Path testLogPath = action.getTestLog().getPath();
@@ -191,6 +197,19 @@ public class WorkerTestStrategy extends StandaloneTestStrategy {
     }
   }
 
+  private static Map<String, String> addPersistentRunnerVars(Map<String, String> originalEnv)
+      throws UserExecException {
+    if (originalEnv.containsKey("PERSISTENT_TEST_RUNNER")) {
+      throw new UserExecException(
+          "Found clashing environment variable with persistent_test_runner."
+              + " Please use another test strategy");
+    }
+    return ImmutableMap.<String, String>builder()
+        .putAll(originalEnv)
+        .put("PERSISTENT_TEST_RUNNER", "true")
+        .build();
+  }
+
   private List<String> getStartUpArgs(TestRunnerAction action) throws ExecException {
     List<String> args = getArgs(/*coverageScript=*/ "", action);
     ImmutableList.Builder<String> startupArgs = ImmutableList.builder();
@@ -198,8 +217,6 @@ public class WorkerTestStrategy extends StandaloneTestStrategy {
     startupArgs.add(args.get(0)).add("--no_echo");
     // Add remaining of the original args.
     startupArgs.addAll(args.subList(1, args.size()));
-    // Make the Test runner run persistently.
-    startupArgs.add("--persistent_test_runner");
     // Add additional flags requested for this invocation.
     startupArgs.addAll(MoreObjects.firstNonNull(
             extraFlags.get(action.getMnemonic()), ImmutableList.<String>of()));
diff --git a/src/test/shell/bazel/bazel_java_test.sh b/src/test/shell/bazel/bazel_java_test.sh
index 1ce5184edd..6076ea86e0 100755
--- a/src/test/shell/bazel/bazel_java_test.sh
+++ b/src/test/shell/bazel/bazel_java_test.sh
@@ -169,52 +169,55 @@ EOF
 
 function test_java_test_main_class() {
   mkdir -p java/testrunners || fail "mkdir failed"
-  cat > java/testrunners/TestRunner.java <<EOF
-package testrunners;
 
-import com.google.testing.junit.runner.BazelTestRunner;
+  cat > java/testrunners/TestRunnerPass.java <<EOF
+package testrunners;
 
-public class TestRunner {
+public class TestRunnerPass {
   public static void main(String[] argv) {
-    System.out.println("Custom test runner was run");
-    BazelTestRunner.main(argv);
+    System.out.println("Custom test runner pass was run");
   }
 }
 EOF
 
-  cat > java/testrunners/Tests.java <<EOF
+  cat > java/testrunners/TestRunnerFail.java <<EOF
 package testrunners;
 
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-import org.junit.Test;
-
-@RunWith(JUnit4.class)
-public class Tests {
-
-  @Test
-  public void testTest() {
-    System.out.println("testTest was run");
+public class TestRunnerFail {
+  public static void main(String[] argv) {
+    System.out.println("Custom failing test runner was run");
+    System.exit(1); // signal failure
   }
 }
 EOF
 
   cat > java/testrunners/BUILD <<EOF
-java_library(name = "test_runner",
-             srcs = ['TestRunner.java'],
-             deps = ['@bazel_tools//tools/jdk:TestRunner_deploy.jar'],
+java_library(name = "test_runner_pass",
+             srcs = ['TestRunnerPass.java'],
+)
+
+java_library(name = "test_runner_fail",
+             srcs = ['TestRunnerFail.java'],
+)
+
+java_test(name = "TestsPass",
+          main_class = "testrunners.TestRunnerPass",
+          runtime_deps = [':test_runner_pass']
 )
 
-java_test(name = "Tests",
-          srcs = ['Tests.java'],
-          deps = ['@bazel_tools//tools/jdk:TestRunner_deploy.jar'],
-          main_class = "testrunners.TestRunner",
-          runtime_deps = [':test_runner']
+java_test(name = "TestsFail",
+          main_class = "testrunners.TestRunnerFail",
+          runtime_deps = [':test_runner_fail']
 )
 EOF
-  bazel test --test_output=streamed //java/testrunners:Tests &> "$TEST_log"
-  expect_log "Custom test runner was run"
-  expect_log "testTest was run"
+
+  bazel test --test_output=streamed //java/testrunners:TestsPass &> "$TEST_log" \
+    || fail "Unexpected failure"
+  expect_log "Custom test runner pass was run"
+
+  bazel test --test_output=streamed //java/testrunners:TestsFail &> "$TEST_log" \
+    && fail "Expected failure" || true
+  expect_log "Custom failing test runner was run"
 }
 
 function test_basic_java_sandwich() {
@@ -433,7 +436,10 @@ java_test(
   name = "MainTest",
   size = "small",
   srcs = ["MainTest.java"],
-  deps = [":custom"]
+  deps = [
+           ":custom",
+           '@bazel_tools//tools/jdk:TestRunner_deploy.jar',
+         ]
 )
 
 java_custom_library(
diff --git a/src/test/shell/bazel/persistent_test_runner_test.sh b/src/test/shell/bazel/persistent_test_runner_test.sh
index 8af6d04a65..deb2d501d0 100755
--- a/src/test/shell/bazel/persistent_test_runner_test.sh
+++ b/src/test/shell/bazel/persistent_test_runner_test.sh
@@ -22,7 +22,7 @@ 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; }
 
-function DISABLED_test_simple_scenario() {
+function test_simple_scenario() {
   mkdir -p java/testrunners || fail "mkdir failed"
 
   cat > java/testrunners/TestsPass.java <<EOF
@@ -80,8 +80,7 @@ EOF
       || true
 }
 
-# TODO(kush): Enable this test once we're able to reload modified classes in persistent test runner.
-function DISABLED_test_reload_modified_classes() {
+function test_reload_modified_classes() {
   mkdir -p java/testrunners || fail "mkdir failed"
 
   # Create a passing test.