[SUREFIRE-1710] skipAfterFailureCount in JUnit5 provider

[Tibor17]

This is my old change from my shelf. Providing it open. The ITs are broken and the impl should be fixed. Implements the config parameter for JUnit5 skipAfterFailure.

Following this checklist to help us incorporate your contribution quickly and easily:

• [x] Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
• [x] Each commit in the pull request should have a meaningful subject line and body.
• [x] Format the pull request title like [SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles, where you replace SUREFIRE-XXX with the appropriate JIRA issue. Best practice is to use the JIRA issue title in the pull request title and in the first line of the commit message.
• [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• [ ] Run mvn clean install to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
• [ ] You have run the integration tests successfully (mvn -Prun-its clean install).

If your pull request is about ~20 lines of code you don't need to sign an Individual Contributor License Agreement if you are unsure please ask on the developers list.

To make clear that you license your contribution under the Apache License Version 2.0, January 2004 you have to acknowledge this by using the following check-box.

• [x] I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

• [x] In any other case, please file an Apache Individual Contributor License Agreement.

[Tibor17]

@andyrokit @mpkorstanje This is the implementation for https://issues.apache.org/jira/browse/SUREFIRE-1743 and https://issues.apache.org/jira/browse/SUREFIRE-1710 but it still does not work properly. Whatever value is set in skipAfterFailure, it still stops the execution with 2 failures. If you guys have a guess, pls ping us.

[Tibor17]

The class FailFastJUnit5IT should be renamed...

[mpkorstanje]

Cheers. I don't mind taking a look but at a glance this will only work with JUnit Jupiter rather then every TestEngine on the JUnit Platform. The current implementation is tied to JUnit Jupiter APIs.

It won't work for example with JUnit 4 (through JUnit 5s Vintage module) or Cucumber.

[mpkorstanje]

Overall I think it would be more effective to first resolve https://github.com/junit-team/junit5/issues/1880 before trying to make it work in surefire.

[Tibor17]

Yes, it would be worth to have the fix of JUnit5 issue I reported. Unfortunately, the JUnit5 team does not care ;-)

[mpkorstanje]

I believe the JUnit team uses community engagement as a measure of importance and interest.

Now it seems the conversation never moved on after the brainstorming. I think they are waiting for other people - possibly me, you or someone else - to reply to the brainstorm suggestions or provide their own. And then move on towards an implementation.

I've had no problems getting other features merged provided I did the heavy lifting. So I don't think this is a lack of care or interest. It is just not the most important thing compared to everything else. Especially with no or limited apparent community interest.

I do suspect that if someone has a sufficient need or interest in this feature it will be implemented. With that in mind I think it'd be better not to implement this in an ad-hoc fashionen. It will hopefully make some contributor come out of the woodwork.

Or if not, the feature wasn't really needed.

On a personal note: Due to circumstances I've had to significantly reduce the scope of my involvement. I think it is unlikely I'll be able to invest time getting this implemented on the JUnit side.

[Tibor17]

@marcphilipp Hi Marc,

I have tried to implement the feature pleaseStop() by other way than we would expect in https://github.com/junit-team/junit5/issues/1880 I have no idea what's wrong with my two SPIs. But I guess the native support would be booletproof in JUnit5 engines and API.

[marcphilipp]

Overall I think it would be more effective to first resolve junit-team/junit5#1880 before trying to make it work in surefire.

I agree. The only other solution I see is to do this from the Maven JVM, i.e. terminating the fork, instead of doing it in the provider.

[Tibor17]

This feature is implemented in all providers JUnit4 and TestNG. The same principle - we are counting the errors inside and outside of the JVM. If you have one fork then of course you have to count the failures/errors inside. The first happens in the concurrent model that wins and notification is sent and the framework like JUnit4 or TestNG is called with the method pleaseStop() and the framework stops "scheduling" new tests. Stopping the JVM is too naive.

[Vampire]

If this is going to make it into surefire without https://github.com/junit-team/junit5/issues/1880 being fixed and used, I think stuff you called "junit5" should be renamed to "jupiter".

Spock 2+ for example is also a JUnit 5 engine, but would not be handled by your solution as far as I have seen from a very quick look. Probably the same for other engines like Vintage that runs JUnit 4 tests as JUnit 5 engine. At least providing some Jupiter services in common-junit5 makes me think it only supports the Jupiter engine and should thus be common-jupiter. :-)