2nd attempt to split classpaths, and load test classes in a separate …

…classloader. Currently this funcionality is hidden behind the "experimental_testrunner" flag.

Original description (from commit 786cfa2):
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 (and having the tag "experimental_testrunner" in the java_test rule).
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)

--
PiperOrigin-RevId: 151065529
MOS_MIGRATED_REVID=151065529

src/java_tools/junitrunner/java/com/google/testing/junit/runner/ExperimentalTestRunner.java

@@ -23,14 +23,16 @@
 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;
 import java.util.concurrent.TimeUnit;
 
 /**
- * For now the same as {@link BazelTestRunner} but intended to be a testbed to try out new features,
- * without breeaking existing tests.
+ * Like {@link BazelTestRunner} but runs the tests in their own classloader.
  */
 public class ExperimentalTestRunner {
   /**
@@ -39,6 +41,9 @@ public class ExperimentalTestRunner {
    */
   static final String TEST_SUITE_PROPERTY_NAME = "bazel.test_suite";
 
+  private static URL[] classpaths = null;
+  private static URLClassLoader targetClassLoader;
+
   private ExperimentalTestRunner() {
     // utility class; should not be instantiated
   }
@@ -118,7 +123,7 @@ private static boolean checkTestSuiteProperty(String testSuiteProperty) {
   }
 
   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
@@ -128,16 +133,33 @@ private static int runTestsInSuite(String suiteClassName, String[] args) {
       }
     }
 
-    // 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;
   }
 
+  /**
+   * 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;
@@ -149,11 +171,10 @@ private static int runPersistentTestRunner(String suiteClassName) {
         if (request == null) {
           break;
         }
-        ByteArrayOutputStream baos = new ByteArrayOutputStream();
-        PrintStream ps = new PrintStream(baos, true);
-        System.setOut(ps);
-        System.setErr(ps);
-
+        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 {
@@ -164,7 +185,11 @@ private static int runPersistentTestRunner(String suiteClassName) {
         }
 
         WorkResponse response =
-            WorkResponse.newBuilder().setOutput(baos.toString()).setExitCode(exitCode).build();
+            WorkResponse
+                .newBuilder()
+                .setOutput(outputStream.toString())
+                .setExitCode(exitCode)
+                .build();
         response.writeDelimitedTo(System.out);
         System.out.flush();
 
@@ -176,18 +201,54 @@ private static int runPersistentTestRunner(String suiteClassName) {
     return 0;
   }
 
-  private static Class<?> getTestClass(String name) {
-    if (name == null) {
+  /**
+   * 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.

src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java

@@ -34,7 +34,9 @@
 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;
@@ -256,18 +258,27 @@ public Artifact createStubAction(
     arguments.add(Substitution.of("%needs_runfiles%",
         ruleContext.getFragment(Jvm.class).getJavaExecutable().isAbsolute() ? "0" : "1"));
 
-    NestedSetBuilder<Artifact> classpathBuilder = NestedSetBuilder.naiveLinkOrder();
     TransitiveInfoCollection testSupport = getTestSupport(ruleContext);
-    if (testSupport != null) {
-      // Currently, this is only needed when experimental_testrunner=true, since in other cases the
-      // testSupport classpath is already present in the javaCommon.getRuntimeClasspath().
-      classpathBuilder.addTransitive(getRuntimeJarsForTargets(testSupport));
+    NestedSet<Artifact> classpath = javaCommon.getRuntimeClasspath();
+    NestedSet<Artifact> testTargetClasspath = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+    if (TargetUtils.isTestRule(ruleContext.getRule())
+        && getMainClassFromRule(ruleContext).equals(EXPERIMENTAL_TEST_RUNNER_MAIN_CLASS)) {
+      // Experimental testRunner needs the testSupport to be present as it needs to start with
+      // *only* the testSupport classpaths.
+      Preconditions.checkNotNull(testSupport);
+      // 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);
     }
-    classpathBuilder.addTransitive(javaCommon.getRuntimeClasspath());
-
     arguments.add(
         new ComputedClasspathSubstitution(
-            "%classpath%", classpathBuilder.build(), workspacePrefix, isRunfilesEnabled));
+            "%classpath%", classpath, workspacePrefix, isRunfilesEnabled));
+    arguments.add(
+        new ComputedClasspathSubstitution(
+            "%test_target_classpath%", testTargetClasspath, workspacePrefix, isRunfilesEnabled));
 
     JavaCompilationArtifacts javaArtifacts = javaCommon.getJavaCompilationArtifacts();
     String path =

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.

src/test/shell/bazel/persistent_test_runner_test.sh

@@ -83,8 +83,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() {
   setup_javatest_support
   mkdir -p java/testrunners || fail "mkdir failed"
 
@@ -109,6 +108,7 @@ EOF
   cat > java/testrunners/BUILD <<EOF
 java_test(name = "Tests",
           srcs = ['Tests.java'],
+          tags = ["experimental_testrunner"],
           deps = ['//third_party:junit4'],
 )
 EOF