stop adding test support classes to junit failure report

Review Request #3620 — Created March 27, 2016 and submitted

cheister
pants
fix-junit-failure-report
3102
pants-reviews
gmalmquist, zundel

Stop adding test support classes to junit failure report

Travis CI: https://travis-ci.org/pantsbuild/pants/builds/118836767

Fix the JUnit failure report so it finds failures from tests in inner classes and stops adding test support classes to the failure report.

Before a failure in the JUnit tests would print

                     FAILURES!!!
                     Tests run: 77,  Failures: 1


FAILURE: 
    tests/java/org/pantsbuild/args4j:args4j
        org.pantsbuild.args4j.ArgfileOptionHandlerTest
        org.pantsbuild.args4j.BooleanOptionHandlerTest
        org.pantsbuild.args4j.CollectionOptionHandlerTest
        org.pantsbuild.args4j.ParserTest

    tests/java/org/pantsbuild/testing:testing
        org.pantsbuild.testing.EasyMockTestTest

    tests/java/org/pantsbuild/tools/jar:jar
        org.pantsbuild.tools.jar.JarBuilderTest
        org.pantsbuild.tools.jar.JarEntryCopierTest
        org.pantsbuild.tools.jar.OptionsTest

    tests/java/org/pantsbuild/tools/junit/impl:junit
        org.pantsbuild.tools.junit.impl.ConsoleRunnerTest#testNormalTesting
        org.pantsbuild.tools.junit.impl.ConsoleRunnerTestBase

    tests/java/org/pantsbuild/tools/runner:jar
        org.pantsbuild.tools.runner.PantsRunnerTest

And will now print only the failing test

                     FAILURES!!!
                     Tests run: 77,  Failures: 1


FAILURE: 
    tests/java/org/pantsbuild/tools/junit/impl:junit
        org.pantsbuild.tools.junit.impl.ConsoleRunnerTest#testNormalTesting
  1. 
      
  2. Why this change?

    1. If you run ./pants test tests/java:: you can see the naming of the xml files for tests defined in inner classes.

      e.g

      .pants.d/test/junit/TEST-org.pantsbuild.tools.jar.JarBuilderTest-DuplicateHandlerTests-AlwaysTest.xml
      .pants.d/test/junit/TEST-org.pantsbuild.tools.jar.JarBuilderTest-DuplicateHandlerTests-DuplicateHandlerTest.xml
      .pants.d/test/junit/TEST-org.pantsbuild.tools.jar.JarBuilderTest-DuplicatePolicyTests-DelegateTest.xml
      .pants.d/test/junit/TEST-org.pantsbuild.tools.jar.JarBuilderTest-DuplicatePolicyTests-MatchesTest.xml
      

      But the classnames for these tests are

      org.pantsbuild.tools.jar.JarBuilderTest$DuplicateHandlerTests$AlwaysTest
      org.pantsbuild.tools.jar.JarBuilderTest$DuplicateHandlerTests$DuplicateHandlerTest
      org.pantsbuild.tools.jar.JarBuilderTest$DuplicatePolicyTests$DelegateTest
      org.pantsbuild.tools.jar.JarBuilderTest$DuplicatePolicyTests$MatchesTest
      

      This change converts the tests with inner classes to potential xml filenames output by the ConsoleTestRunner.

  3. The case this is meant to detect is when a test fails in a class initializer. I agree it had a lot of false positives, but it would be great to still report that problem. I don't see a test for it and I'm wondering if this logic will blow up in a different way if the xml_filename computed above doesn't exist.

    1. This code shouldn't blow up from a missing xml file because the xml_filename is only added to the xml_filenames_to_targets dictionary if it exists on the filesystem.

      The case of an initialization failure not creating a TEST-*.xml file was specifically fixed in https://rbcommons.com/s/twitter/r/3571/. If we find more cases of missing xml report files we should fix the xml reporting in ConsoleTestRunner so we can rely on the xml reports being accurate here.

      There are a lot of cases where the test classes used in the JUnit test runner do not have TEST-*.xml files created for them. The example in the summary for this change shows how many of the xml files do not exist just for the Java test classes in Pants. Trying to figure out which ones are support classes and which ones are missing tests classes should be validated in another way rather than printing warnings about a lot of missing xml files that should not exist anyway.

      Maybe we could parse the Failures: <num> from the JUnit output and warn if it does not match the number of failures we're reporting in the failure summary?

  4. 
      
  1. Ship It!
  2. 
      
  1. 
      
  2. 
      
  1. Ship It!
  2. 
      
  1. Landed in 9d16240c327a5803cb017d83165dd460fbb8f472, please mark as submitted.

  2. 
      
Review request changed

Status: Closed (submitted)

Loading...