If that's not currently possible, maybe a SPI should be provided so that people can use plug in extensions to analyze the test result and override it if necessary (transforming an error into a warning, storing results in a way which is easier to use by other tools later...) ?
Guillaume Le lun. 14 mars 2022 à 07:43, Christoph Läubrich <[email protected]> a écrit : > I also agree that the test at least should run, we use this property to > run the test and produce result and later on have a buildstep that > analyze the results (and probably fail the build job). > > As it is not recommend, I wonder what is the recommended way to archive > something similar? > > Am 14.03.22 um 06:29 schrieb Olivier Lamy: > > On Mon, 14 Mar 2022 at 11:55, Tibor Digana <[email protected]> > 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 > >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > -- ------------------------ Guillaume Nodet
