Remove unneeded args4j handler registrations that cause failures in tests and rename TestParser

Review Request #3583 - Created March 18, 2016 and submitted

Information
Chris Heisterkamp
pants
remove-args4j-registerhandler-calls
3059
Reviewers
pants-reviews
zundel
Remove unneeded args4j handler registrations that cause failures in tests and rename TestParser

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

This fixes the problem seen in https://rbcommons.com/s/twitter/r/3571/ and https://rbcommons.com/s/twitter/r/2406/ and recorded in https://github.com/pantsbuild/pants/issues/1727

It's hard to reproduce the problem in an isolated test but in https://rbcommons.com/s/twitter/r/3571/ it will always happen if you change the order of arguments in ConsoleRunnerTestHelper#runTestAndReturnXmlFile and move the -xmlreport flag before the -outdir flag then run ./pants test tests/java::

It seems to be a problem when the Parser runs OptionHandlerRegistry.getRegistry().registerHandler(boolean.class, BooleanOptionHandler.class); in the ParserTest code and then any ConsoleRunnerTests are run with the -outdir change above. The error is

1) testXmlReportAllPassing(org.pantsbuild.tools.junit.impl.ConsoleRunnerTest)
                     java.lang.RuntimeException: Classloading error during test discovery for /var/folders/cc/7wn2nwjn1gd4cd08fpp3zb6r00015s/T/junit1339494313910125036/testOutputDir
                        at org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.notFoundError(ConsoleRunnerImpl.java:613)
                        at org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.parseRequests(ConsoleRunnerImpl.java:500)
                        at org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.run(ConsoleRunnerImpl.java:372)
                        at org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.main(ConsoleRunnerImpl.java:753)
                        at org.pantsbuild.tools.junit.impl.ConsoleRunnerTestHelper.runTestAndReturnXmlFile(ConsoleRunnerTestHelper.java:71)
                        at org.pantsbuild.tools.junit.impl.ConsoleRunnerTestHelper.runTestAndParseXml(ConsoleRunnerTestHelper.java:79)
                        at org.pantsbuild.tools.junit.impl.ConsoleRunnerTest.testXmlReportAllPassing(ConsoleRunnerTest.java:194)
                        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
                        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
                        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
                        at java.lang.reflect.Method.invoke(Method.java:498)
                        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
                        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
                        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
                        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
                        at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
                        at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
                        at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
                        at org.junit.rules.RunRules.evaluate(RunRules.java:20)
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.withretry.BlockJUnit4ClassRunnerWithRetry$InvokeWithRetry.evaluate(BlockJUnit4ClassRunnerWithRetry.java:57)
                        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
                        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
                        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
                        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
                        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
                        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
                        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
                        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
                        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.CompositeRequest.runChild(CompositeRequest.java:66)
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.ConcurrentCompositeRequest$1$1.run(ConcurrentCompositeRequest.java:36)
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.ConcurrentRunnerScheduler.finished(ConcurrentRunnerScheduler.java:85)
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.ConcurrentCompositeRequest$1.evaluate(ConcurrentCompositeRequest.java:45)
                        at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
                        at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.run(ConsoleRunnerImpl.java:420)
                        at __shaded_by_pants__.org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.main(ConsoleRunnerImpl.java:753)
                        at org.pantsbuild.tools.junit.ConsoleRunner.main(ConsoleRunner.java:12)

Which seems to indicate that the ConsoleRunnerImpl parser is not pulling the -outdir directory off of the command line and is instead trying to interpret it as a test class with tests. When ConsoleRunnerImpl tries to load the file to look for test methods it fails with the error above.

Both ParserTest and ConsoleRunnerTest pass when run in isolation so it seems to indicate that some global state is not getting cleaned up between the tests. The OptionHandlerRegistry.getRegistry() is global singleton in args4j and it looks like leaving the pants version BooleanOptionHandler registered will mess up any other tests that run after the ParserTest and do not want the different functionality.

Eric Ayers
Eric Ayers
Chris Heisterkamp
Eric Ayers
Eric Ayers
Chris Heisterkamp
Stu Hood
Chris Heisterkamp
Eric Ayers
Chris Heisterkamp
Review request changed

Status: Closed (submitted)

Loading...