Fix handling of method specs.

Review Request #4258 - Created Sept. 23, 2016 and submitted

Information
John Sirois
pants
jsirois/issues/3837/fix_missing_test_handling
3902
0068ff7...
Reviewers
pants-reviews
stuhood, wisechengyi, zundel
Previously the method spec handling code did not guard against non-test
classes and would throw an NPE. This change adds a failing test and
fixes the handling to deal with `Optional`. The `Spec` class is made
immutable in the process to improve the code's ability to be reasoned
about.

 3rdparty/jvm/org/hamcrest/BUILD                                  | 17 +++++++++
 src/java/org/pantsbuild/tools/junit/impl/ConsoleRunnerImpl.java  | 15 +++-----
 src/java/org/pantsbuild/tools/junit/impl/Spec.java               | 40 +++++++++++++---------
 src/java/org/pantsbuild/tools/junit/impl/SpecException.java      |  8 ++---
 src/java/org/pantsbuild/tools/junit/impl/SpecParser.java         | 93 ++++++++++++++++++++++----------------------------
 testprojects/tests/java/org/pantsbuild/testproject/matcher/BUILD |  1 +
 tests/java/org/pantsbuild/tools/junit/impl/BUILD                 |  7 ++++
 tests/java/org/pantsbuild/tools/junit/impl/SpecParserTest.java   | 55 +++++++++++++++--------------
 tests/java/org/pantsbuild/tools/junit/impl/SpecTest.java         | 13 +++----
 tests/java/org/pantsbuild/tools/junit/lib/BUILD                  |  2 +-
 10 files changed, 135 insertions(+), 116 deletions(-)

I noticed this issue when attempting the following:

./pants test.junit \
  --no-timeouts \
  --open \
  --coverage-open \
  --test=tests/java/org/pantsbuild/args4j/ParserTest.java#test{Failure,Success} \
  tests/java/org/pantsbuild/args4j/::
...
11:26:39 00:03       [run]
                     OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=256m; support was removed in 8.0
                     OpenJDK 64-Bit Server VM warning: ignoring option UseSplitVerifier; support was removed in 8.0
                     Auto-detected 8 processors, using -parallel-threads=8
                     Exception in thread "main" java.lang.NullPointerException
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.SpecParser.addMethod(SpecParser.java:130)
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.SpecParser.parse(SpecParser.java:61)
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.run(ConsoleRunnerImpl.java:455)
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.main(ConsoleRunnerImpl.java:845)
                        at org.pantsbuild.tools.junit.ConsoleRunner.main(ConsoleRunner.java:12)

                   Test failed: 
                   java org.pantsbuild.tools.junit.ConsoleRunner ... exited non-zero (1); 0 failed 0 targets.
...

Locally green using ./pants test-changed.

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

John Sirois
Yi Cheng
John Sirois
John Sirois
John Sirois
Eric Ayers
John Sirois
John Sirois
Eric Ayers
Stu Hood
John Sirois
John Sirois
John Sirois
Review request changed

Status: Closed (submitted)

Change Summary:

Now on master:

git log -1 origin/master
commit f46e0702f4409d982b79cab9a3789b35090d46a4
Author: John Sirois <john.sirois@gmail.com>
Date:   Mon Sep 26 20:01:03 2016 -0600

    Fix handling of method specs.
    
    Previously the method spec handling code did not guard against non-test
    classes and would throw an NPE. This change adds a failing test and
    fixes the handling to deal with `Optional`. The `Spec` class is made
    immutable in the process to improve the code's ability to be reasoned
    about.
    
    Testing Done:
    I noticed this issue when attempting the following:
    ```
    ./pants test.junit \
      --no-timeouts \
      --open \
      --coverage-open \
      --test=tests/java/org/pantsbuild/args4j/ParserTest.java#test{Failure,Success} \
      tests/java/org/pantsbuild/args4j/::
    ...
    11:26:39 00:03       [run]
                         OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=256m; support was removed in 8.0
                         OpenJDK 64-Bit Server VM warning: ignoring option UseSplitVerifier; support was removed in 8.0
                         Auto-detected 8 processors, using -parallel-threads=8
                         Exception in thread "main" java.lang.NullPointerException
                            at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.SpecParser.addMethod(SpecParser.java:130)
                            at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.SpecParser.parse(SpecParser.java:61)
                            at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.run(ConsoleRunnerImpl.java:455)
                            at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.main(ConsoleRunnerImpl.java:845)
                            at org.pantsbuild.tools.junit.ConsoleRunner.main(ConsoleRunner.java:12)
    
                       Test failed:
                       java org.pantsbuild.tools.junit.ConsoleRunner ... exited non-zero (1); 0 failed 0 targets.
    ...
    ```
    
    Locally green using `./pants test-changed`.
    
    CI went green here:
      https://travis-ci.org/pantsbuild/pants/builds/162902557
    
    Bugs closed: 3902
    
    Reviewed at https://rbcommons.com/s/twitter/r/4258/
Loading...