Handle skipped tests in TAP reporter #262
Conversation
|
Ok, tests for the TAP reporter output added, including a test for the skip directive. This should increase test coverage, though the coveralls job in https://travis-ci.org/hadley/testthat/builds/64178838 timed out (which appears to be why the Travis build was marked as failing; the tests passed). |
tests/testthat/test-reporter-tap.r
Outdated
There was a problem hiding this comment.
I think you could simplify this with capture.output()
There was a problem hiding this comment.
Yes, thanks, much nicer.
|
Could you please also add a bullet point to NEWS? |
|
Thanks! (For future reference, I don't get a notification when you make changes, just when you make comments, so once you're done, it's helpful to add a comment like "PTAL") |
|
Ah ok, thanks for pointing that out, will do. |
|
Looks like the NEWS bullet I added was clobbered in a merge: nealrichardson/testthat@master...hadley:master#diff-8312ad0561ef661716b48d09478362f3L10 I could make a new PR with that if you want, but maybe it's easier for you to just add it back. |
Because
skipped tests did not haveresult$passed, they were marked as "not ok" in the TAP output. Following http://testanything.org/tap-specification.html, I added a skip directive to the output in case ofresult$skipped.I'd happily write a test for this change if you could indicate where that should be. I didn't find any tests for the reporter output, only for the existence of reporters. I did confirm that the output with the skip directives is correctly parsed, at least by the Jenkins TAP parser: