Patch to make scala tests work

Change to make scalatest test work using the JUnitRunner builtin to scala test.  Basically I check if the test extends org.scalatest.Suite, if it does I include it in the specs.  Later on, we do the same thing when converting the test class to a junit Runner.  We check to see if it is a Suite, and if so then fake like we included the @RunWith annotation.

A few notes:
  1) I didn't support scala test "methods"
  2) I used reflection instead of modifying junit.py and friends to make scalatest be a non-shaded jar (in fact we don't need to include it at all)
  3) I made sure I used the test class's class loader to load Suite and JUnitRunner
  4) I bumped the runner_jar version to 1.0.16, I hope this is correct.

I would love to see this as a 1.2.1 release.

NOTE:  This is a recreate of pantsbuild#4344

Bugs closed: 4013

Reviewed at https://rbcommons.com/s/twitter/r/4361/

src/java/org/pantsbuild/tools/junit/impl/AnnotatedClassRequest.java

@@ -8,8 +8,6 @@
 import org.junit.internal.requests.ClassRequest;
 import org.junit.runner.Runner;
 
-import org.pantsbuild.tools.junit.withretry.AllDefaultPossibilitiesBuilderWithRetry;
-
 /**
  * A ClassRequest that exposes the wrapped class. Also used to support retrying
  * flaky tests via AllDefaultPossibilitiesBuilderWithRetry, that in turn gives us
@@ -40,6 +38,6 @@ public Class<?> getClazz() {
   @Override
   public Runner getRunner() {
     return new
-        AllDefaultPossibilitiesBuilderWithRetry(numRetries, err).safeRunnerForClass(testClass);
+        CustomAnnotationBuilder(numRetries, err).safeRunnerForClass(testClass);
   }
 }

src/java/org/pantsbuild/tools/junit/impl/ConsoleRunnerImpl.java

@@ -49,7 +49,6 @@
 import org.pantsbuild.junit.annotations.TestParallel;
 import org.pantsbuild.junit.annotations.TestSerial;
 import org.pantsbuild.tools.junit.impl.experimental.ConcurrentComputer;
-import org.pantsbuild.tools.junit.withretry.AllDefaultPossibilitiesBuilderWithRetry;
 
 /**
  * An alternative to {@link JUnitCore} with stream capture and junit-report xml output capabilities.
@@ -520,10 +519,14 @@ private int runExperimental(Collection<Spec> parsedTests, JUnitCore core)
     return failures;
   }
 
-  private int runConcurrentTests(JUnitCore core, SpecSet specSet, Concurrency concurrency) {
+  private int runConcurrentTests(JUnitCore core, SpecSet specSet, Concurrency concurrency)
+      throws InitializationError {
     Computer junitComputer = new ConcurrentComputer(concurrency, parallelThreads);
     Class<?>[] classes = specSet.extract(concurrency).classes();
-    return core.run(junitComputer, classes).getFailureCount();
+    CustomAnnotationBuilder builder =
+        new CustomAnnotationBuilder(numRetries, swappableErr.getOriginal());
+    Runner suite = junitComputer.getSuite(builder, classes);
+    return core.run(Request.runner(suite)).getFailureCount();
   }
 
   private int runLegacy(Collection<Spec> parsedTests, JUnitCore core) throws InitializationError {
@@ -573,7 +576,12 @@ private List<Request> legacyParseRequests(PrintStream err, Collection<Spec> spec
       if (this.perTestTimer || this.parallelThreads > 1) {
         for (Class<?> clazz : classes) {
           if (legacyShouldRunParallelMethods(clazz)) {
-            testMethods.addAll(TestMethod.fromClass(clazz));
+            if (ScalaTestUtil.isScalaTestTest(clazz)) {
+              // legacy and scala doesn't work easily.  just adding the class
+              requests.add(new AnnotatedClassRequest(clazz, numRetries, err));
+            } else {
+              testMethods.addAll(TestMethod.fromClass(clazz));
+            }
           } else {
             requests.add(new AnnotatedClassRequest(clazz, numRetries, err));
           }
@@ -583,8 +591,8 @@ private List<Request> legacyParseRequests(PrintStream err, Collection<Spec> spec
         // requests.add(Request.classes(classes.toArray(new Class<?>[classes.size()])));
         // does, except that it instantiates our own builder, needed to support retries.
         try {
-          AllDefaultPossibilitiesBuilderWithRetry builder =
-              new AllDefaultPossibilitiesBuilderWithRetry(numRetries, err);
+          CustomAnnotationBuilder builder =
+              new CustomAnnotationBuilder(numRetries, err);
           Runner suite = new Computer().getSuite(
               builder, classes.toArray(new Class<?>[classes.size()]));
           requests.add(Request.runner(suite));

src/java/org/pantsbuild/tools/junit/impl/CustomAnnotationBuilder.java

@@ -0,0 +1,58 @@
+// Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
+// Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+package org.pantsbuild.tools.junit.impl;
+
+import java.io.PrintStream;
+
+import org.junit.internal.builders.AllDefaultPossibilitiesBuilder;
+import org.junit.internal.builders.AnnotatedBuilder;
+import org.junit.internal.builders.JUnit4Builder;
+import org.junit.runner.Runner;
+import org.junit.runners.model.RunnerBuilder;
+import org.pantsbuild.tools.junit.withretry.JUnit4BuilderWithRetry;
+
+/**
+ * Needed to support retrying flaky tests as well as add support for running scala tests.
+ * Using method overriding, gives us access to code in JUnit4 that cannot be customized
+ * in a simpler way.
+ */
+public class CustomAnnotationBuilder extends AllDefaultPossibilitiesBuilder {
+
+  private final int numRetries;
+  private final PrintStream err;
+
+  public CustomAnnotationBuilder(int numRetries, PrintStream err) {
+    super(true);
+    this.numRetries = numRetries;
+    this.err = err;
+  }
+
+  @Override
+  public JUnit4Builder junit4Builder() {
+    return new JUnit4BuilderWithRetry(numRetries, err);
+  }
+
+  // override annotated builder to "fake" the scala test junit runner for scala tests
+  @Override
+  protected AnnotatedBuilder annotatedBuilder() {
+    return new ScalaTestAnnotatedBuilder(this);
+  }
+
+  private static class ScalaTestAnnotatedBuilder extends AnnotatedBuilder {
+    ScalaTestAnnotatedBuilder(RunnerBuilder suiteBuilder) {
+      super(suiteBuilder);
+    }
+
+    @Override
+    public Runner runnerForClass(Class<?> testClass) throws Exception {
+      Runner runner = super.runnerForClass(testClass);
+      if (runner == null) {
+        if (ScalaTestUtil.isScalaTestTest(testClass)) {
+          return ScalaTestUtil.getJUnitRunner(testClass);
+        }
+      }
+      return runner;
+    }
+  }
+}

src/java/org/pantsbuild/tools/junit/impl/ScalaTestUtil.java

@@ -0,0 +1,40 @@
+package org.pantsbuild.tools.junit.impl;
+
+import org.junit.runner.Runner;
+
+public final class ScalaTestUtil {
+  private ScalaTestUtil() {}
+
+  /**
+   * Returns a scalatest junit runner using reflection in the classloader of the test.
+   * @param clazz the test class
+   *
+   * @return a new scala test junit runner
+   */
+  public static Runner getJUnitRunner(Class<?> clazz) {
+    try {
+      Class<?> junitRunnerClass = Class.forName("org.scalatest.junit.JUnitRunner",
+          true, clazz.getClassLoader());
+      return (Runner)junitRunnerClass.getConstructor(Class.class).newInstance(clazz);
+    } catch (Exception e) {
+      // isScalaTest should fail if scala test isn't available so this is probably ok.
+      throw new RuntimeException(e);
+    }
+  }
+
+  /**
+   * Checks if the passed in test clazz has an ancestor that is the scala test suite
+   * object (looked up in the test classes class loader).
+   * @param clazz the test class
+   *
+   * @return true if the test class is a scalatest test, false if not.
+   */
+  public static boolean isScalaTestTest(Class<?> clazz) {
+    try {
+      Class suiteClass = Class.forName("org.scalatest.Suite", true, clazz.getClassLoader());
+      return suiteClass.isAssignableFrom(clazz);
+    } catch (ClassNotFoundException e) {
+      return false;
+    }
+  }
+}

src/java/org/pantsbuild/tools/junit/impl/Util.java

@@ -5,15 +5,16 @@
 
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
-import java.util.Arrays;
 import org.junit.Ignore;
 import org.junit.runner.Description;
 import org.junit.runner.RunWith;
 import org.junit.runner.notification.Failure;
 
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.Arrays;
+
 /**
  * Utilities for working with junit test runs.
  */
@@ -170,6 +171,10 @@ public static boolean isTestClass(final Class<?> clazz) {
       return true;
     }
 
+    if (ScalaTestUtil.isScalaTestTest(clazz)) {
+      return true;
+    }
+
     // Support junit 4.x @Test annotated methods.
     return Iterables.any(Arrays.asList(clazz.getMethods()), IS_ANNOTATED_TEST_METHOD);
   }

tests/java/org/pantsbuild/tools/junit/impl/BUILD

@@ -12,6 +12,7 @@ junit_tests(
     'src/java/org/pantsbuild/junit/annotations',
     'src/java/org/pantsbuild/tools/junit',
     'tests/java/org/pantsbuild/tools/junit/lib:test-dep',
+    'tests/scala/org/pantsbuild/tools/junit/lib:scala-test-dep',
   ],
   sources=globs('*Test.java'),
   timeout=180,

tests/java/org/pantsbuild/tools/junit/impl/ConsoleRunnerTest.java

@@ -344,6 +344,12 @@ public void testMockRunWithTest() throws Exception {
     assertEquals("mrwt1-bar mrwt1-foo", TestRegistry.getCalledTests());
   }
 
+  @Test
+  public void testMockScalaTest() throws Exception {
+    invokeConsoleRunner("MockScalaTest");
+    assertEquals("MockScalaTest-1", TestRegistry.getCalledTests());
+  }
+
   @Test
   public void testNotATestNoPublicConstructor() throws Exception {
     // This class contains no public constructor. The test runner should ignore
@@ -386,6 +392,13 @@ public void testNotATestInterface() throws Exception {
     assertEquals("", TestRegistry.getCalledTests());
   }
 
+  @Test
+  public void testNotATestScalaClass() throws Exception {
+    // This class is a basic scala class, test runner should ignore
+    invokeConsoleRunner("NotAScalaTest");
+    assertEquals("", TestRegistry.getCalledTests());
+  }
+
   @Test
   public void testLegacyConcurrencyOptions() {
     // New style option overrides old

tests/java/org/pantsbuild/tools/junit/impl/SpecParserTest.java

@@ -12,6 +12,7 @@
 import org.junit.Test;
 import org.pantsbuild.tools.junit.lib.MockJUnit3Test;
 import org.pantsbuild.tools.junit.lib.MockRunWithTest;
+import org.pantsbuild.tools.junit.lib.MockScalaTest;
 import org.pantsbuild.tools.junit.lib.UnannotatedTestClass;
 
 import static org.hamcrest.CoreMatchers.containsString;
@@ -136,4 +137,12 @@ private void assertNoSpecs(String className) throws Exception {
     Spec spec = Iterables.getOnlyElement(specs);
     assertEquals(MockRunWithTest.class, spec.getSpecClass());
   }
+
+  @Test public void testScalaTest() throws Exception {
+    String specString = "org.pantsbuild.tools.junit.lib.MockScalaTest";
+    SpecParser parser = new SpecParser(ImmutableList.of(specString));
+    Collection<Spec> specs = parser.parse();
+    Spec spec = Iterables.getOnlyElement(specs);
+    assertEquals(MockScalaTest.class, spec.getSpecClass());
+  }
 }

tests/java/org/pantsbuild/tools/junit/impl/UtilTest.java

@@ -7,7 +7,9 @@
 import org.junit.runner.Description;
 import org.pantsbuild.tools.junit.lib.MockJUnit3Test;
 import org.pantsbuild.tools.junit.lib.MockRunWithTest;
+import org.pantsbuild.tools.junit.lib.MockScalaTest;
 import org.pantsbuild.tools.junit.lib.MockTest1;
+import org.pantsbuild.tools.junit.lib.NotAScalaTest;
 import org.pantsbuild.tools.junit.lib.NotATestAbstractClass;
 import org.pantsbuild.tools.junit.lib.NotATestInterface;
 import org.pantsbuild.tools.junit.lib.NotATestNoPublicConstructor;
@@ -101,12 +103,14 @@ public void testIsATestClass() {
     assertTrue(Util.isTestClass(MockJUnit3Test.class));
     assertTrue(Util.isTestClass(MockRunWithTest.class));
     assertTrue(Util.isTestClass(UnannotatedTestClass.class));
+    assertTrue(Util.isTestClass(MockScalaTest.class));
     assertFalse(Util.isTestClass(NotATestAbstractClass.class));
     assertFalse(Util.isTestClass(NotATestNonzeroArgConstructor.class));
     assertFalse(Util.isTestClass(NotATestNoPublicConstructor.class));
     assertFalse(Util.isTestClass(NotATestInterface.class));
     assertFalse(Util.isTestClass(NotATestNoRunnableMethods.class));
     assertFalse(Util.isTestClass(NotATestPrivateClass.class));
+    assertFalse(Util.isTestClass(NotAScalaTest.class));
 
     // Even though this is ignored it should still be considered a Test
     assertTrue(Util.isTestClass(XmlReportIgnoredTestSuiteTest.class));

tests/scala/org/pantsbuild/tools/junit/lib/BUILD

@@ -0,0 +1,11 @@
+# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+scala_library(
+  name='scala-test-dep',
+  dependencies=[
+    '3rdparty:scalatest',
+    'tests/java/org/pantsbuild/tools/junit/lib:test-dep',
+  ],
+  sources=globs('*.scala')
+)

tests/scala/org/pantsbuild/tools/junit/lib/MockScalaTest.scala

@@ -0,0 +1,11 @@
+package org.pantsbuild.tools.junit.lib
+
+import org.scalatest.FreeSpec
+
+class MockScalaTest extends FreeSpec {
+  "test" - {
+    "should pass" in {
+      TestRegistry.registerTestCall("MockScalaTest-1")
+    }
+  }
+}

tests/scala/org/pantsbuild/tools/junit/lib/NotAScalaTest.scala

@@ -0,0 +1,3 @@
+package org.pantsbuild.tools.junit.lib
+
+class NotAScalaTest {}