Report JUnit tests with failing assumptions as skipped tests

Review Request #4010 — Created June 15, 2016 and submitted

cheister
pants
report-assumption-failures
3587
pants-reviews
stuhood, zundel
Report JUnit tests with failing assumptions as skipped tests

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

  • 0
  • 0
  • 1
  • 4
  • 5
Description From Last Updated
  1. 
      
  2. Is there any case where testSuite.getTime() would be 0?

    1. If all the tests in the testSuite are skipped then the time would be 0.

  3. 
      
  1. Ship It!
  2. 
      
  1. 
      
  2. should this have an else w/ something like the null case for testFailure?

    Perhaps we could also finish the suite here like testIgnored. Would that make sense?

  3. Same question, but for the testCase portion.

  4. nit: add the copyright header.

  5. 
      
  1. 
      
  2. It was modeled after testIgnored rather than testFailure since an assumptionFailure means the test will be Ignored.

    Most of the null checks in the callbacks are not exercised by the tests. I suspect most of the null checks are not necessary. I would rather wait to add them here until after we have tests that demonstrate they are needed.

    We should not need to finish the testSuite because you will still get the the testStarted and testFinished callbacks for a test with an assumption failure.

    1. @nickhoward, up until recently, we had very little test coverage for the JUnit runner. No doubt some of these defensive null checks were put in out of an abundance of caution because there was no telling if we had broken the runner. But today the case is vastly different. Not only do we have good test coverage, but Chris refactored the runner and xml report writing and fixed a lot of long standing problems. If someone can come up with a scenario that breaks this, we owe it to ourselves to add a test that exercises the scenario.

    2. Sounds good to me.

  3. Same comment as above. We should add them if we have tests that show they are needed.

  4. 
      
  1. Ship It!
  2. 
      
  1. Ship It!
  2. 
      
  1. Merged to master @ 0347708

  2. 
      
Review request changed

Status: Closed (submitted)

Loading...