Fix mvn surefire config: better test output #1289
Conversation
|
Nice! Do you know why the test count is different now? |
It seems to be due to the change surefire-junit3 -> surefire-junit47. @Tibor17 may have some clue |
|
BTW if I run org.junit.tests.AllTests with IDEA, by clicking on the class name and selecting Run test (thus bypassing mvn), I get the same result as the one I get with surefire-junit47. That's good news, I think. Also, about the skipped tests, I've found a bug: With this we're now at Then I see there is something wrong with this test: (in AssumptionTest) It's being marked as skipped. Again, I'll open a separate issue. |
In case a test is expecting AssumptionViolatedException by using the attribute 'expected' of the @test annotation, the test will actually be marked as skipped by IDEA and mvn, as if the AssumptionViolatedException had been thrown up in the stack. See issue junit-team#1290
|
Please let me know if you prefer 6413e99 to be in a separate PR |
| Object[] objects = {1, 2, null}; | ||
| assumeNotNull(objects); | ||
| try { | ||
| Object[] objects = {1, 2, null}; |
There was a problem hiding this comment.
Please put this line outside of the "try" block
There was a problem hiding this comment.
I am personally not a fan of using the "expected" attribute of @Test. It doesn't allow you to specify which line should throw the exception, and as you've seen it can have bugs (the one you found wasn't the first)
There was a problem hiding this comment.
True.
+1
Il 13/mag/2016 14:56, "Kevin Cooney" notifications@github.com ha scritto:
In src/test/java/org/junit/tests/experimental/AssumptionTest.java:
public void assumeNotNullThrowsException() {
Object[] objects = {1, 2, null};assumeNotNull(objects);try {Object[] objects = {1, 2, null};I am personally not a fan of using the "expected" attribute of @test. It
doesn't allow you to specify which line should throw the exception, and as
you've seen it can have bugs (the one you found wasn't the first)—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
|
I have a slight preference for 6413e99 as a separate pull. Is it too much trouble? |
Of course not. Will do that now. |
Use a try/catch block instead of the attribute 'expected' of the @test annotation. See bug junit-team#1290. See discussion on junit-team#1289.
|
I really like this and would like to merge it. @kcooney Any objections? |
|
@marcphilipp no objections. |
|
@alb-i986 Thanks again! |
Mvn test output is now more detailed and helpful.
Before:
After:
Also, in case of a failing test, the output was pretty bad:
https://travis-ci.org/alb-i986/junit/jobs/129797694
But now:
https://travis-ci.org/alb-i986/junit/jobs/129797962
The only downside, as you can see, is that it's not so easy to see the stack trace for the failing test, buried among the tens of test classes. I think it should rather be reported at the very end, as it was with the old surefire junit3 provider. But this is a UX bug of surefire.
Please note that the test count is a bit different now.
Before:
After: