Copy junit-runner in from twitter/commons.

Review Request #2043 — Created April 7, 2015 and submitted

jsirois
pants
jsirois/jvm_tools/adopt_junit_runner
1362, 1379
c2139fb...
pants-reviews
benjyw, stuhood, zundel
This ports the junit-runner used by pants but does not actually
configure pants to use the ported runner.  That will come later after
the tool is published: https://github.com/pantsbuild/pants/issues/1361

The junit-runner in twitter/commons will be deleted after this tool is
published and consumed by pants HEAD.

 3rdparty/BUILD                                                                             |   5 +
 BUILD                                                                                      |   5 +-
 src/java/org/pantsbuild/junit/annotations/BUILD                                            |  12 +
 src/java/org/pantsbuild/junit/annotations/TestParallel.java                                |  21 ++
 src/java/org/pantsbuild/junit/annotations/TestSerial.java                                  |  21 ++
 src/java/org/pantsbuild/tools/junit/AbortableListener.java                                 |  75 ++++++
 src/java/org/pantsbuild/tools/junit/AnnotatedClassRequest.java                             |  45 ++++
 src/java/org/pantsbuild/tools/junit/AntJunitXmlReportListener.java                         | 445 +++++++++++++++++++++++++++++++
 src/java/org/pantsbuild/tools/junit/BUILD                                                  |  36 +++
 src/java/org/pantsbuild/tools/junit/CompositeRequest.java                                  |  79 ++++++
 src/java/org/pantsbuild/tools/junit/ConcurrentCompositeRequest.java                        |  49 ++++
 src/java/org/pantsbuild/tools/junit/ConcurrentRunnerScheduler.java                         |  99 +++++++
 src/java/org/pantsbuild/tools/junit/ConsoleListener.java                                   |  26 ++
 src/java/org/pantsbuild/tools/junit/ConsoleRunner.java                                     | 712 ++++++++++++++++++++++++++++++++++++++++++++++++++
 src/java/org/pantsbuild/tools/junit/ForwardingListener.java                                | 103 ++++++++
 src/java/org/pantsbuild/tools/junit/ListenerRegistry.java                                  |  19 ++
 src/java/org/pantsbuild/tools/junit/PerClassConsoleListener.java                           |  33 +++
 src/java/org/pantsbuild/tools/junit/StreamSource.java                                      |  30 +++
 src/java/org/pantsbuild/tools/junit/Util.java                                              |  50 ++++
 src/java/org/pantsbuild/tools/junit/withretry/AllDefaultPossibilitiesBuilderWithRetry.java |  30 +++
 src/java/org/pantsbuild/tools/junit/withretry/BUILD                                        |  15 ++
 src/java/org/pantsbuild/tools/junit/withretry/BlockJUnit4ClassRunnerWithRetry.java         |  84 ++++++
 src/java/org/pantsbuild/tools/junit/withretry/JUnit4BuilderWithRetry.java                  |  30 +++
 tests/java/org/pantsbuild/tools/junit/BUILD                                                |  11 +
 tests/java/org/pantsbuild/tools/junit/ConsoleRunnerTest.java                               | 111 ++++++++
 tests/java/org/pantsbuild/tools/junit/FlakyTest.java                                       |  99 +++++++
 tests/java/org/pantsbuild/tools/junit/MockTest1.java                                       |  24 ++
 tests/java/org/pantsbuild/tools/junit/MockTest2.java                                       |  19 ++
 tests/java/org/pantsbuild/tools/junit/MockTest3.java                                       |  19 ++
 tests/java/org/pantsbuild/tools/junit/README                                               |  55 ++++
 tests/java/org/pantsbuild/tools/junit/TestRegistry.java                                    |  55 ++++
 31 files changed, 2415 insertions(+), 2 deletions(-)

I dogfooded the runner to test itself via:

$ pants.dev clean-all test {src,tests}/java/::

And by using the instructions in
tests/java/org/pantsbuild/tools/junit/README with the following diff:

$ git diff
diff --git a/BUILD.tools b/BUILD.tools
index fc53e75..da0a936 100644
--- a/BUILD.tools
+++ b/BUILD.tools
@@ -123,7 +123,8 @@ jar_library(name = 'junit',
             jars = [
               jar(org = 'junit', name = 'junit-dep', rev = '4.10'),
               jar(org = 'org.hamcrest', name = 'hamcrest-core', rev = '1.2'),
-              jar(org = 'com.twitter.common', name = 'junit-runner', rev = '0.0.41'),
+              jar(org = 'org.pantsbuild', name = 'junit-runner', rev = 'none', mutable = True,
+                  url = 'file:///home/jsirois/dev/3rdparty/jsirois-pants4/dist/junit-runner.jar'),

               # TODO(Eric Ayers) We need to rename/shade the dependencies of junit-runner
               #      or use a custom classloader for junit runner to permanently
diff --git a/src/python/pants/backend/jvm/tasks/junit_run.py b/src/python/pants/backend/jvm/tasks/junit_run.py
index 0c48798..509bf26 100644
--- a/src/python/pants/backend/jvm/tasks/junit_run.py
+++ b/src/python/pants/backend/jvm/tasks/junit_run.py
@@ -649,7 +649,7 @@ class Cobertura(_Coverage):


 class JUnitRun(JvmTask, JvmToolTaskMixin):
-  _MAIN = 'com.twitter.common.junit.runner.ConsoleRunner'
+  _MAIN = 'org.pantsbuild.tools.junit.ConsoleRunner'

   @classmethod
   def register_options(cls, register):

CI went green here:
https://travis-ci.org/pantsbuild/pants/builds/57658118

JS
  1. Please hold off for diff 2 before reviewing.
    
    Diff 1 is large, but a straight copy from twitter/commons mod mechanical package re-names.
    Diff 2 is small and eliminates all compile warnings.
  2. 
      
JS
JS
JS
BE
  1. Looks good! Just a couple of minor stlye nits.

  2. Space between catch and opening paren.

  3. Use 2-space indents in this file, not 4-space, for consistency.

    1. This is using Twitter [1]/Goog [2]style - 2 space indent, 4 space line continuation indent on the whole, although I over-continuation-indented by 2 for 3 lines in the second constructor - fixed those from 6 to 4.
      
      [1] https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/styleguide.md#indent-style
      [2] https://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s4.2-block-indentation
          https://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s4.5.2-line-wrapping-indent
  4. 
      
JS
BE
  1. Ship It!
  2. 
      
JS
ZU
  1. Just some random comments.

    I saw the comments that it is difficult to test this library under pants. Does src/java get compiled at all during the CI run? It wasn't clear to me reading build-support/bin/ci.sh

    1. Good catch!
      
      The README comments are about integration testing / dogfooding, not unit testing.  That said I did mean to augment ci.sh and forgot - now fixed.
  2. 3rdparty/BUILD (Diff revision 4)
     
     

    We've talked about it before, but I thought I would just bring it up again to keep the idea fresh. We don't use args4j, and it only pulls in junit, so this shouldn't cause any issues with conflicting deps but on principle I've got an eye toward expunging 3rdparty dependencies from the runner or otherwise isolate them from the test code.

    1. Fresher than you know!: https://github.com/jsirois/pants/commit/9580ab78044d844bb2079f01e28bbf128716fc7a
      I'm trying to get basic automatic jvm tool shading reviews all out today.  No support for user-specified shading rules or any of that to start.
  3. Is parallel testing in wide use at Twitter?

    1. Yes.  This was a contrib from an ads developer and they used it in their codebase to drop test times for all of ads code by 30+% iirc.  This was a big deal because as I recall their total runtime on a clean build was something like 40% compile and 60% test.
  4. Aside: No action item here, but this kind of scaling has been problematic for us running in lightweight containers that don't actually change the # of availableProcessors(). It might be helpful to print out what this number is when automatically tuned.

  5. Rename to README.md?

    1. Its not in markdown form so I'll defer.
  6. 
      
JS
ZU
  1. Ship It!
  2. 
      
JS
  1. Thanks guys - submitted @ https://github.com/pantsbuild/pants/commit/9844090b3d2c89c408db8d2b36a43409a0076577
  2. 
      
JS
Review request changed

Status: Closed (submitted)

Loading...