From 6e60b0389814d8361e453092d3b18f52c3e4bcb1 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Sun, 27 Feb 2022 17:15:23 +1000 Subject: [PATCH] [SUREFIRE-1426] Fork crash doesn't fail build with -Dmaven.test.failure.ignore=true remove not needed local variable Signed-off-by: Olivier Lamy fix temporary debug Signed-off-by: Olivier Lamy [SUREFIRE-1426] Fork crash doesn't fail build with -Dmaven.test.failure.ignore=true, add an IT which show it looks to be fixed with 3.0.0-M6 but was failing with 3.0.0-M5 proposal fix in case of SurefireBooterException (i.e cannot start surefire fork) error must be reported Signed-off-by: Olivier Lamy --- .../maven/plugin/surefire/SurefireHelper.java | 10 +++- .../surefire/AbstractSurefireMojoTest.java | 5 +- .../plugin/surefire/SurefireHelperTest.java | 42 ++++++++++++++++ ...efire1426JvmCrashShouldNotBeIgnoredIT.java | 47 ++++++++++++++++++ .../pom.xml | 49 +++++++++++++++++++ .../src/test/java/PojoTest.java | 44 +++++++++++++++++ 6 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1426JvmCrashShouldNotBeIgnoredIT.java create mode 100644 surefire-its/src/test/resources/surefire-1426-ignore-fail-jvm-crash/pom.xml create mode 100644 surefire-its/src/test/resources/surefire-1426-ignore-fail-jvm-crash/src/test/java/PojoTest.java diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/SurefireHelper.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/SurefireHelper.java index 1870223f55..5822451046 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/SurefireHelper.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/SurefireHelper.java @@ -27,6 +27,7 @@ import org.apache.maven.surefire.api.cli.CommandLineOption; import org.apache.maven.surefire.api.suite.RunResult; import org.apache.maven.surefire.api.testset.TestSetFailedException; +import org.apache.maven.surefire.booter.SurefireBooterForkException; import javax.annotation.Nonnull; import java.io.File; @@ -156,7 +157,14 @@ public static void reportExecution( SurefireReportParameters reportParameters, R if ( reportParameters.isTestFailureIgnore() ) { - log.error( createErrorMessage( reportParameters, result, firstForkException ) ); + String errorMessage = createErrorMessage( reportParameters, result, firstForkException ); + + if ( firstForkException instanceof SurefireBooterForkException ) + { + throw new MojoExecutionException( errorMessage, firstForkException ); + } + + log.error( errorMessage ); } else { diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoTest.java index 9d89447aff..8bcf947203 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoTest.java @@ -2000,6 +2000,7 @@ public static class Mojo private List includes; private List excludes; private String test; + private boolean testFailureIgnore; private JUnitPlatformProviderInfo createJUnitPlatformProviderInfo( Artifact junitPlatformArtifact, TestClassPath testClasspathWrapper ) @@ -2075,13 +2076,13 @@ public void setSkip( boolean skip ) @Override public boolean isTestFailureIgnore() { - return false; + return testFailureIgnore; } @Override public void setTestFailureIgnore( boolean testFailureIgnore ) { - + this.testFailureIgnore = testFailureIgnore; } @Override diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/SurefireHelperTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/SurefireHelperTest.java index 1ecaea7435..dbe2c3697b 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/SurefireHelperTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/SurefireHelperTest.java @@ -19,12 +19,18 @@ * under the License. */ +import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; import org.apache.maven.plugin.surefire.AbstractSurefireMojoTest.Mojo; +import org.apache.maven.plugin.surefire.log.PluginConsoleLogger; import org.apache.maven.surefire.api.suite.RunResult; +import org.apache.maven.surefire.api.testset.TestSetFailedException; +import org.apache.maven.surefire.booter.SurefireBooterForkException; +import org.codehaus.plexus.logging.Logger; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.ArgumentCaptor; import java.io.File; import java.util.ArrayList; @@ -36,7 +42,13 @@ import static org.apache.maven.plugin.surefire.SurefireHelper.reportExecution; import static org.apache.maven.surefire.shared.lang3.SystemUtils.IS_OS_WINDOWS; import static org.assertj.core.api.Assertions.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.junit.Assume.assumeTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.powermock.api.mockito.PowerMockito.doNothing; +import static org.powermock.api.mockito.PowerMockito.mock; /** * Test of {@link SurefireHelper}. @@ -120,6 +132,36 @@ public void shouldEscapeWindowsPath() assertThat( escaped ).isEqualTo( root + "\\" + pathToJar ); } + @Test + public void shouldHandleFailWithoutExitCode() throws Exception + { + RunResult summary = new RunResult( 0, 0, 0, 0 ); + Mojo plugin = new Mojo(); + plugin.setTestFailureIgnore( true ); + + Logger logger = mock( Logger.class ); + when( logger.isErrorEnabled() ).thenReturn( true ); + doNothing().when( logger ).error( anyString() ); + TestSetFailedException exc = new TestSetFailedException( "failure" ); + reportExecution( plugin, summary, new PluginConsoleLogger( logger ), exc ); + ArgumentCaptor errorMessage = ArgumentCaptor.forClass( String.class ); + verify( logger ).error( errorMessage.capture() ); + assertThat( errorMessage.getValue() ).contains( "failure" ); + } + + @Test + public void shouldHandleFailIfJvmNonZeroExitCode() throws Exception + { + RunResult summary = new RunResult( 0, 0, 0, 0 ); + Mojo plugin = new Mojo(); + plugin.setTestFailureIgnore( true ); + + SurefireBooterForkException exc = new SurefireBooterForkException( "Unrecognized option: -Xxxx" ); + e.expect( MojoExecutionException.class ); + e.expectMessage( containsString( "Unrecognized option: -Xxxx" ) ); + reportExecution( plugin, summary, new PluginConsoleLogger( mock( Logger.class ) ), exc ); + } + @Test public void shouldHandleFailIfNoTests() throws Exception { diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1426JvmCrashShouldNotBeIgnoredIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1426JvmCrashShouldNotBeIgnoredIT.java new file mode 100644 index 0000000000..5a575abbe7 --- /dev/null +++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1426JvmCrashShouldNotBeIgnoredIT.java @@ -0,0 +1,47 @@ +package org.apache.maven.surefire.its.jiras; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import org.apache.maven.it.VerificationException; +import org.apache.maven.surefire.its.fixture.SurefireJUnit4IntegrationTestCase; +import org.junit.Test; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; + +/** + * Test https://issues.apache.org/jira/browse/SUREFIRE-1426 + * + */ +public class Surefire1426JvmCrashShouldNotBeIgnoredIT + extends SurefireJUnit4IntegrationTestCase +{ + @Test + public void mavenShouldFail() throws VerificationException + { + unpack( "surefire-1426-ignore-fail-jvm-crash" ) + .maven() + .withFailure() + .debugLogging() + .executeTest() + .assertThatLogLine( containsString( "BUILD SUCCESS" ), is( 0 ) ) + .verifyTextInLog( "BUILD FAILURE" ); + } +} diff --git a/surefire-its/src/test/resources/surefire-1426-ignore-fail-jvm-crash/pom.xml b/surefire-its/src/test/resources/surefire-1426-ignore-fail-jvm-crash/pom.xml new file mode 100644 index 0000000000..103a39c1ba --- /dev/null +++ b/surefire-its/src/test/resources/surefire-1426-ignore-fail-jvm-crash/pom.xml @@ -0,0 +1,49 @@ + + + + + 4.0.0 + + org.apache.maven.plugins.surefire + SUREFIRE-1426 + 1.0-SNAPSHOT + SUREFIRE-1426 + + + 1.8 + 1.8 + + + + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + -Dfile.encoding=UTF-8 -Duser.language=en -XFFOOOBEEER -Duser.region=US -showversion -Xmx6g -Xms2g -XX:+PrintGCDetails + true + + + + + diff --git a/surefire-its/src/test/resources/surefire-1426-ignore-fail-jvm-crash/src/test/java/PojoTest.java b/surefire-its/src/test/resources/surefire-1426-ignore-fail-jvm-crash/src/test/java/PojoTest.java new file mode 100644 index 0000000000..18fad81681 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-1426-ignore-fail-jvm-crash/src/test/java/PojoTest.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +public class PojoTest +{ + private static int calls; + + public void setUp() + { + System.out.println( "setUp called " + ++calls ); + } + + public void tearDown() + { + System.out.println( "tearDown called " + calls ); + } + + public void testSuccess() + { + assert true; + } + + public void testFailure() + { + assert false; + } + +}