Log exceptions from testRunFinished() in our listener

Review Request #4060 — Created July 10, 2016 and submitted

Eric Ayers
pants
zundel/debug-when-test-run-failure-throws
3638, 3645
c574f09...
pants-reviews
benjyw, gmalmquist, stuhood

Mimics an issue report in https://github.com/pantsbuild/pants/issues/3638 where a failure from testRunFinished() in a listener aborts the test but otherwise gives no error messages.

In the scenario we saw in the field, an operating system error creating a file caused the tests to not failures, but no indication why. Before this change, pants would return something like:

E
Time: 0

OK 
(0 tests)
ConsoleRunner exited with status 1

After this change, we at least print the exception to the console:

java.io.IOException: Bogus IOException
    at org.pantsbuild.tools.junit.impl.ConsoleRunnerTest$1AbortInTestRunFinishedListener.testRunFinished(ConsoleRunnerTest.java:424)
    at org.pantsbuild.tools.junit.impl.ForwardingListener$2.fire(ForwardingListener.java:54)
    at org.pantsbuild.tools.junit.impl.ForwardingListener.fire(ForwardingListener.java:37)
    at org.pantsbuild.tools.junit.impl.ForwardingListener.testRunFinished(ForwardingListener.java:52)
    at org.pantsbuild.tools.junit.impl.AbortableListener.testRunFinished(AbortableListener.java:32)
    at org.junit.runner.notification.SynchronizedRunListener.testRunFinished(SynchronizedRunListener.java:42)
    at org.junit.runner.notification.RunNotifier$2.notifyListener(RunNotifier.java:103)
    at org.junit.runner.notification.RunNotifier$SafeNotifier.run(RunNotifier.java:72)
    at org.junit.runner.notification.RunNotifier.fireTestRunFinished(RunNotifier.java:100)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:138)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
    at org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.runLegacy(ConsoleRunnerImpl.java:516)
    at org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.run(ConsoleRunnerImpl.java:429)
    at org.pantsbuild.tools.junit.impl.ConsoleRunnerImpl.main(ConsoleRunnerImpl.java:811)
    at org.pantsbuild.tools.junit.lib.ConsoleRunnerTestBase.invokeConsoleRunner(ConsoleRunnerTestBase.java:129)
    at org.pantsbuild.tools.junit.impl.ConsoleRunnerTest.testRunFinishFailed(ConsoleRunnerTest.java:431)

...
Time: 0.011

OK 
(0 tests)
ConsoleRunner exited with status 1

Manually verified that debug output is being written in this case when running tests in IJ.

In the future when a new test runner is published, we could add an integration test that verifies that these messages make it to the console.

CI green at https://travis-ci.org/pantsbuild/pants/builds/143723517

Chris Heisterkamp
  1. 
      
  2. So the throwable just gets swallowed somewhere else?

    1. It is actually caught in JUnit's RunNotifier. There is a failure accounted for in the test run, but no output ever makes it to the screen. My theory is that this is becuase the listener that outputs to the console (ConsoleListener or PerTestConsoleListener) has already run at this point. I will experiment with moving the order of registering these listeners.

  3. Should this be aded to core instead of abortableListener?

    1. If we did that, the code in abortableListener couldn't catch the exceptoin.

  4. It doesn't seem necessary to do this in both the tearDown and the setUp. The setUp call should be sufficient.

  5. 
      
Stu Hood
  1. 
      
  2. It doesn't look like this does anything that will cause this to blame the appropriate test; would be good to confirm that this ends up in the test result XML from the run.

    1. There is no single test to blame. This is the end of the test run when our runner is dumping all of the stuff its stored in memory out to an XML file. Its the xml file itself that can't be written.

  3. 
      
Stu Hood
  1. Ship It!
  2. 
      
Eric Ayers
Review request changed

Status: Closed (submitted)

Change Summary:

Commit d79a7a3. If I get a chance I'll still experiment around with moving the order of listener registration.

Loading...