On Mon, 14 Mar 2022 at 11:55, Tibor Digana <tibordig...@apache.org> wrote:
> In case of the user property *maven.test.failure.ignore* the MOJO must not > throw any exception which is interpreted by the Maven Core as BUILD > SUCCESS. > This is a very simple reduction of the problem description. The documentation here https://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#testFailureIgnore says "Set this to "true" to ignore a failure during testing. Its use is NOT RECOMMENDED, but quite convenient on occasion" Personally, I understand this to ignore failures in junit results BUT at least I want the tests to run. I guess this is how our users use this feature (feature we do not recommend by the way...) And this is perfectly explained here https://issues.apache.org/jira/browse/SUREFIRE-1426?focusedCommentId=16188077&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16188077 there is a difference between ignoring some junit failures and ignoring a configuration error because some jvm args has been misconfigured for many reasons and surefire cannot start. If I follow your reasoning ("MOJO must not throw any exception ") we should ignore every surefire configuration error and keep running the build until the end and says BUILD SUCCESS what about mvn test -Dsurefire.rerunFailingTestsCount=notanumber -Dmaven.test.failure.ignore=true we should not throw any exceptions as you said below and keep saying BUILD SUCCESS? argLine is a configuration element as any others so it should fail directly and not be ignored especially when the surefire plugin cannot run. > We have received an internal requirement to change the behavior of the user > property *maven.test.failure.ignore* so that the behavior will have one > exception. Suppose that you have JDK 1.8 but you use: > /jre/java --add-reads ... > The outcome is JVM exit with an error message. > I agree with Herve who also says that *maven.test.failure.ignore* should > not allow the MOJO to throw the exception. It is not a typical JVM > segmentation fault or another JVM crash where we cannot do anything about > it, and where the entire build would crash in the command line which > of course means that the build cannot normally be interpreted as BUILD > SUCCESS. So we are still on the same level of failures related to the test > purposes. > > On the other hand, Olivier has reopened the issues SUREFIRE-1426 and > SUREFIRE-1681 where the exceptional behavior of the feature is expected. > This is exactly the reason why I closed these tickets several years ago and > my proposal was to not to have any exceptions in the feature behavior and > the proposal was to introduce a new user property for exact use cases. > The next idea, which comes from two developers, would provide us with the > same non exceptional and exact behavior of the user property > *maven.test.failure.ignore* but it would also provide us with new user > property in the case with fine grade control of the build errors, e.g. > *maven.test.jvm.error.ignore*. > with a default of? honestly I just see this new parameter as introducing more complexity in an already very complex code and not worth it. again read both issue's comments and my comments. Please try to have a user POV and think about making our users' experience more simple. This should be very simple: If surefire forked jvm cannot start it's build error and cannot run any tests, it's a problem users want to know immediately because it can be for a lot of reasons: wrong argLine, not enough memory on the system etc... AND AGAIN it is very different from ignoring junit result failures. Try to look at a user point of view and think about what is the use case of the property maven.test.failure.ignore=true, I guess 99% of the time, users wants to run all their tests (even on a CI with different matrix) so they can collect all the tests results which has runned to see if there is any issue for some combination of their matrix and eventually turn the result as unstable (this is a very typical use case in Jenkins and was even a built in feature of the previous Jenkins Maven plugin) BUT if for any reasons one of the module do not start his tests because the jvm cannot be start the users will not see that and will be totally blind until maybe someone look inside a very very large log file (which means probably never) Long story short as my experience as a user facing this problem/bug: I had the case on a very large multi modules (~250 modules) build of a very used open source project. I was using this maven.test.failure.ignore property but one of the modules had a bad jpms configuration for a jdk17 profile on the CI. The build has a matrix of 2 os and 4 jdks and different maven run which means around ~60k tests and a Jenkins log file about 40M So because of this property the build was not failing and kept saying BUILD SUCCESS for weeks/months and basically not testing one module with jdk 17... And frankly you do not dig into a log file of 32M after each run especially when it says success... 3 days after the first release claiming supporting jdk 17 we received a bug report about a something not working with jdk17.... and guess what? Where was this feature supposed to be tested? so I frankly believe we do not need a complex new property, in this case this should fail directly because this will improve user experience. > > > I would like to open the discussion on this topic. You're welcome! > > > Cheers > Tibor >
