Revert "Do not ignore mvntest failure"

[timja]

Reverts #1008

This was incorrect, for the proper implementation see jenkinsci/pipeline-graph-view-plugin#428

Docs update in #1016

linking to #874

[basil]

Yes, it is good to document how to report JUnit test results to Jenkins, and this is ideal. At the same time, I believe few plugins are doing this today (Jenkins core did not do this properly for over a year and a half!), and it is better to fail explicitly rather than to take the risk of merging PRs that break tests due to builds erroneously passing.

I think the best outcome in the short to medium term would be to retain <testFailureIgnore>true</testFailureIgnore> and have plugins opt out (by overridding <testFailureIgnore> to false on CI) as they implement (and test!) JUnit test result XML reporting in Jenkins. That way, the plugin parent POM behavior is consistent with the (current) state of most Jenkins plugins, which do not (yet!) report JUnit XML. When the majority of the ecosystem starts reporting JUnit XML, we can change the default here to match. WDYT?

[timja]

Yes, it is good to document how to report JUnit test results to Jenkins, and this is ideal. At the same time, I believe few plugins are doing this today (Jenkins core did not do this properly for over a year and a half!), and it is better to fail explicitly rather than to take the risk of merging PRs that break tests due to builds erroneously passing.

Few plugins do it either way, most do not configure mvntest at all.
see #1016 (comment)

I think the best outcome in the short to medium term would be to retain <testFailureIgnore>true</testFailureIgnore> and have plugins opt out (by overridding <testFailureIgnore> to false on CI) as they implement (and test!) JUnit test result XML reporting in Jenkins. That way, the plugin parent POM behavior is consistent with the (current) state of most Jenkins plugins, which do not (yet!) report JUnit XML. When the majority of the ecosystem starts reporting JUnit XML, we can change the default here to match. WDYT?

I think that is the safer approach although it was shipped as a semi breaking change to the plugins that do configure it correctly. The user experience is downgraded in those plugins.

I agree that I think we will need plugins to opt-into this to say that they are well configured as the default of changes slipping in without the build failing is not great.

[basil]

I think that is the safer approach although it was shipped as a semi breaking change to the plugins that do configure it correctly. The user experience is downgraded in those plugins.

Agreed, and sorry about that. If you want to implement the opt-in approach, I think that would be ideal; and if not, then we can revert this PR to restore the status quo.