From 11d05413b3104de6b856e7923ee7d327f3cba662 Mon Sep 17 00:00:00 2001 From: andpab Date: Tue, 7 Feb 2023 00:30:35 +0100 Subject: [PATCH] [SUREFIRE-2065] Fix parameterized JUnit4 test reporting (#608) * [SUREFIRE-2065] Fix parameterized JUnit4 test reporting --------- Co-authored-by: Christian Marquez Grabia <4968250+chalmagr@users.noreply.github.com> --- .../maven/surefire/its/fixture/TestFile.java | 19 ++++ .../surefire/its/jiras/Surefire2065IT.java | 103 ++++++++++++++++++ .../resources/surefire-2065-common/pom.xml | 72 ++++++++++++ .../java/pkg/junit4/ParameterizedTest.java | 29 +++++ .../ParameterizedWithDisplayNameTest.java | 31 ++++++ .../java/pkg/junit5/ParameterizedTest.java | 30 +++++ .../ParameterizedWithDisplayNameTest.java | 30 +++++ .../resources/surefire-2065-junit4/pom.xml | 59 ++++++++++ .../java/pkg/junit4/ParameterizedTest.java | 29 +++++ .../ParameterizedWithDisplayNameTest.java | 31 ++++++ .../resources/surefire-2065-junit5/pom.xml | 65 +++++++++++ .../java/pkg/junit5/ParameterizedTest.java | 30 +++++ .../ParameterizedWithDisplayNameTest.java | 30 +++++ .../junitplatform/RunListenerAdapter.java | 35 +++--- 14 files changed, 579 insertions(+), 14 deletions(-) create mode 100644 surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2065IT.java create mode 100644 surefire-its/src/test/resources/surefire-2065-common/pom.xml create mode 100644 surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit4/ParameterizedTest.java create mode 100644 surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit4/ParameterizedWithDisplayNameTest.java create mode 100644 surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit5/ParameterizedTest.java create mode 100644 surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit5/ParameterizedWithDisplayNameTest.java create mode 100644 surefire-its/src/test/resources/surefire-2065-junit4/pom.xml create mode 100644 surefire-its/src/test/resources/surefire-2065-junit4/src/test/java/pkg/junit4/ParameterizedTest.java create mode 100644 surefire-its/src/test/resources/surefire-2065-junit4/src/test/java/pkg/junit4/ParameterizedWithDisplayNameTest.java create mode 100644 surefire-its/src/test/resources/surefire-2065-junit5/pom.xml create mode 100644 surefire-its/src/test/resources/surefire-2065-junit5/src/test/java/pkg/junit5/ParameterizedTest.java create mode 100644 surefire-its/src/test/resources/surefire-2065-junit5/src/test/java/pkg/junit5/ParameterizedWithDisplayNameTest.java diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/TestFile.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/TestFile.java index 88f300208e..2c1fb0f0e4 100644 --- a/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/TestFile.java +++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/fixture/TestFile.java @@ -155,6 +155,25 @@ public TestFile assertContainsText( String text ) return assertContainsText( containsString( text ) ); } + public TestFile assertNotContainsText( Matcher matcher ) + { + final List list = surefireVerifier.loadFile( file, encoding ); + for ( String line : list ) + { + if ( matcher.matches( line ) ) + { + Assert.fail( "Found unexpected message in log" ); + return null; + } + } + return this; + } + + public TestFile assertNotContainsText( String text ) + { + return assertNotContainsText( containsString( text ) ); + } + public URI toURI() { return file.toURI(); diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2065IT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2065IT.java new file mode 100644 index 0000000000..5c6a20a68d --- /dev/null +++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire2065IT.java @@ -0,0 +1,103 @@ +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 static java.nio.charset.StandardCharsets.UTF_8; + +import org.apache.maven.it.VerificationException; +import org.apache.maven.surefire.its.fixture.OutputValidator; +import org.apache.maven.surefire.its.fixture.SurefireJUnit4IntegrationTestCase; +import org.hamcrest.Matchers; +import org.junit.Test; + +/** + * Integration Tests for SUREFIRE-2065 + */ +@SuppressWarnings( "checkstyle:magicnumber" ) +public class Surefire2065IT extends SurefireJUnit4IntegrationTestCase +{ + @Test + public void shouldNotDetectFlakyTestsWhenCombiningJunit4And5Tests() throws VerificationException + { + OutputValidator validator = unpack( "surefire-2065-common" ) + .mavenTestFailureIgnore( true ) + .executeTest() + .assertTestSuiteResults( 8, 0, 4, 0, 0 ); + + assertJunit4( validator ); + assertJunit5( validator ); + } + + @Test + public void shouldNotDetectFlakyTestsWhenRunningOnlyJunit4() throws VerificationException + { + OutputValidator validator = unpack( "surefire-2065-junit4" ) + .mavenTestFailureIgnore( true ) + .executeTest() + .assertTestSuiteResults( 4, 0, 2, 0, 0 ); + + assertJunit4( validator ); + } + + @Test + public void shouldNotDetectFlakyTestsWhenRunningOnlyJunit5() throws VerificationException + { + OutputValidator validator = unpack( "surefire-2065-junit5" ) + .mavenTestFailureIgnore( true ) + .executeTest() + .assertTestSuiteResults( 4, 0, 2, 0, 0 ); + + assertJunit5( validator ); + } + + private static void assertJunit4( OutputValidator validator ) + { + validator.getSurefireReportsFile( "TEST-pkg.junit4.ParameterizedTest.xml", UTF_8 ) + .assertContainsText( Matchers.matchesPattern( "^ *$" ) ) + .assertContainsText( Matchers.matchesPattern( "^ *$" ) ) + .assertContainsText( "$" ) ) + .assertContainsText( Matchers.matchesPattern( "^ *$" ) ) + .assertContainsText( "$" ) ) + .assertContainsText( Matchers.matchesPattern( "^ *$" ) ) + .assertContainsText( "$" ) ) + .assertContainsText( Matchers.matchesPattern( "^ *$" ) ) + .assertContainsText( " + + + + 4.0.0 + + org.example + surefire-2065-common + 1.0-SNAPSHOT + + + UTF-8 + ${java.specification.version} + ${java.specification.version} + 4.12 + 5.3.2 + + + + + junit + junit + ${junit4.version} + test + + + org.junit.jupiter + junit-jupiter-api + ${junit5.version} + test + + + org.junit.jupiter + junit-jupiter-params + ${junit5.version} + test + + + + + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + 1 + + + + + + diff --git a/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit4/ParameterizedTest.java b/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit4/ParameterizedTest.java new file mode 100644 index 0000000000..1456a81385 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit4/ParameterizedTest.java @@ -0,0 +1,29 @@ +package pkg.junit4; + +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class ParameterizedTest +{ + @Parameterized.Parameters + public static List parameters() throws Exception + { + return Arrays.asList( 0, 1 ); + } + + @Parameterized.Parameter(0) + public int expected; + + @Test + public void notFlaky() + { + assertEquals( expected, 0 ); + } +} diff --git a/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit4/ParameterizedWithDisplayNameTest.java b/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit4/ParameterizedWithDisplayNameTest.java new file mode 100644 index 0000000000..af00293609 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit4/ParameterizedWithDisplayNameTest.java @@ -0,0 +1,31 @@ +package pkg.junit4; + +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class ParameterizedWithDisplayNameTest +{ + private static int count = 0; + + @Parameterized.Parameters(name = "value={0}") + public static List parameters() throws Exception + { + return Arrays.asList( 0, 1 ); + } + + @Parameterized.Parameter(0) + public int expected; + + @Test + public void notFlaky() + { + assertEquals( expected, 0 ); + } +} diff --git a/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit5/ParameterizedTest.java b/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit5/ParameterizedTest.java new file mode 100644 index 0000000000..ec2b2fbc57 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit5/ParameterizedTest.java @@ -0,0 +1,30 @@ +package pkg.junit5; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.Arrays; +import java.util.stream.Stream; + +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; + +class ParameterizedTest +{ + static class ParameterSource implements ArgumentsProvider + { + @Override + public Stream provideArguments( ExtensionContext context ) throws Exception + { + return Arrays.asList(0, 1).stream().map( Arguments::of ); + } + } + + @org.junit.jupiter.params.ParameterizedTest + @ArgumentsSource(value = ParameterSource.class) + public void notFlaky(int expected) + { + assertEquals( expected, 0 ); + } +} diff --git a/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit5/ParameterizedWithDisplayNameTest.java b/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit5/ParameterizedWithDisplayNameTest.java new file mode 100644 index 0000000000..94cd8a34a2 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2065-common/src/test/java/pkg/junit5/ParameterizedWithDisplayNameTest.java @@ -0,0 +1,30 @@ +package pkg.junit5; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.Arrays; +import java.util.stream.Stream; + +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; + +class ParameterizedWithDisplayNameTest +{ + static class ParameterSource implements ArgumentsProvider + { + @Override + public Stream provideArguments( ExtensionContext context ) throws Exception + { + return Arrays.asList(0, 1).stream().map( Arguments::of ); + } + } + + @org.junit.jupiter.params.ParameterizedTest(name = "value={0}") + @ArgumentsSource(value = ParameterSource.class) + public void notFlaky(int expected) + { + assertEquals( expected, 0 ); + } +} diff --git a/surefire-its/src/test/resources/surefire-2065-junit4/pom.xml b/surefire-its/src/test/resources/surefire-2065-junit4/pom.xml new file mode 100644 index 0000000000..fa6470cc4c --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2065-junit4/pom.xml @@ -0,0 +1,59 @@ + + + + + 4.0.0 + + org.example + surefire-2065-junit4 + 1.0-SNAPSHOT + + + UTF-8 + ${java.specification.version} + ${java.specification.version} + 4.12 + + + + + junit + junit + ${junit4.version} + test + + + + + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + 1 + + + + + + diff --git a/surefire-its/src/test/resources/surefire-2065-junit4/src/test/java/pkg/junit4/ParameterizedTest.java b/surefire-its/src/test/resources/surefire-2065-junit4/src/test/java/pkg/junit4/ParameterizedTest.java new file mode 100644 index 0000000000..1456a81385 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2065-junit4/src/test/java/pkg/junit4/ParameterizedTest.java @@ -0,0 +1,29 @@ +package pkg.junit4; + +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class ParameterizedTest +{ + @Parameterized.Parameters + public static List parameters() throws Exception + { + return Arrays.asList( 0, 1 ); + } + + @Parameterized.Parameter(0) + public int expected; + + @Test + public void notFlaky() + { + assertEquals( expected, 0 ); + } +} diff --git a/surefire-its/src/test/resources/surefire-2065-junit4/src/test/java/pkg/junit4/ParameterizedWithDisplayNameTest.java b/surefire-its/src/test/resources/surefire-2065-junit4/src/test/java/pkg/junit4/ParameterizedWithDisplayNameTest.java new file mode 100644 index 0000000000..af00293609 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2065-junit4/src/test/java/pkg/junit4/ParameterizedWithDisplayNameTest.java @@ -0,0 +1,31 @@ +package pkg.junit4; + +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class ParameterizedWithDisplayNameTest +{ + private static int count = 0; + + @Parameterized.Parameters(name = "value={0}") + public static List parameters() throws Exception + { + return Arrays.asList( 0, 1 ); + } + + @Parameterized.Parameter(0) + public int expected; + + @Test + public void notFlaky() + { + assertEquals( expected, 0 ); + } +} diff --git a/surefire-its/src/test/resources/surefire-2065-junit5/pom.xml b/surefire-its/src/test/resources/surefire-2065-junit5/pom.xml new file mode 100644 index 0000000000..8241ef480d --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2065-junit5/pom.xml @@ -0,0 +1,65 @@ + + + + + 4.0.0 + + org.example + surefire-2065-junit5 + 1.0-SNAPSHOT + + + UTF-8 + ${java.specification.version} + ${java.specification.version} + 5.3.2 + + + + + org.junit.jupiter + junit-jupiter-api + ${junit5.version} + test + + + org.junit.jupiter + junit-jupiter-params + ${junit5.version} + test + + + + + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + 1 + + + + + + diff --git a/surefire-its/src/test/resources/surefire-2065-junit5/src/test/java/pkg/junit5/ParameterizedTest.java b/surefire-its/src/test/resources/surefire-2065-junit5/src/test/java/pkg/junit5/ParameterizedTest.java new file mode 100644 index 0000000000..ec2b2fbc57 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2065-junit5/src/test/java/pkg/junit5/ParameterizedTest.java @@ -0,0 +1,30 @@ +package pkg.junit5; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.Arrays; +import java.util.stream.Stream; + +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; + +class ParameterizedTest +{ + static class ParameterSource implements ArgumentsProvider + { + @Override + public Stream provideArguments( ExtensionContext context ) throws Exception + { + return Arrays.asList(0, 1).stream().map( Arguments::of ); + } + } + + @org.junit.jupiter.params.ParameterizedTest + @ArgumentsSource(value = ParameterSource.class) + public void notFlaky(int expected) + { + assertEquals( expected, 0 ); + } +} diff --git a/surefire-its/src/test/resources/surefire-2065-junit5/src/test/java/pkg/junit5/ParameterizedWithDisplayNameTest.java b/surefire-its/src/test/resources/surefire-2065-junit5/src/test/java/pkg/junit5/ParameterizedWithDisplayNameTest.java new file mode 100644 index 0000000000..94cd8a34a2 --- /dev/null +++ b/surefire-its/src/test/resources/surefire-2065-junit5/src/test/java/pkg/junit5/ParameterizedWithDisplayNameTest.java @@ -0,0 +1,30 @@ +package pkg.junit5; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.Arrays; +import java.util.stream.Stream; + +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; + +class ParameterizedWithDisplayNameTest +{ + static class ParameterSource implements ArgumentsProvider + { + @Override + public Stream provideArguments( ExtensionContext context ) throws Exception + { + return Arrays.asList(0, 1).stream().map( Arguments::of ); + } + } + + @org.junit.jupiter.params.ParameterizedTest(name = "value={0}") + @ArgumentsSource(value = ParameterSource.class) + public void notFlaky(int expected) + { + assertEquals( expected, 0 ); + } +} diff --git a/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java b/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java index 3e4e605e53..3c519d437a 100644 --- a/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java +++ b/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java @@ -33,7 +33,6 @@ import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.regex.Pattern; import java.util.stream.Stream; import org.apache.maven.surefire.api.report.OutputReportEntry; @@ -61,8 +60,6 @@ final class RunListenerAdapter implements TestExecutionListener, TestOutputReceiver, RunModeSetter { - private static final Pattern COMMA_PATTERN = Pattern.compile( "," ); - private final ClassMethodIndexer classMethodIndexer = new ClassMethodIndexer(); private final ConcurrentMap testStartTime = new ConcurrentHashMap<>(); private final ConcurrentMap failures = new ConcurrentHashMap<>(); @@ -147,7 +144,7 @@ public void executionFinished( TestIdentifier testIdentifier, TestExecutionResul break; case FAILED: String reason = safeGetMessage( testExecutionResult.getThrowable().orElse( null ) ); - SimpleReportEntry reportEntry = createReportEntry( testIdentifier, testExecutionResult, + SimpleReportEntry reportEntry = createReportEntry( testIdentifier, testExecutionResult, reason, elapsed ); if ( isAssertionError ) { @@ -320,7 +317,9 @@ private String[] toClassMethodName( TestIdentifier testIdentifier ) MethodSource methodSource = testSource.map( MethodSource.class::cast ).get(); String realClassName = methodSource.getClassName(); - String[] source = testPlan.getParent( testIdentifier ) + String[] source = collectAllTestIdentifiersInHierarchy( testIdentifier ) + .filter( i -> i.getSource().map( ClassSource.class::isInstance ).orElse( false ) ) + .findFirst() .map( this::toClassMethodName ) .map( s -> new String[] { s[0], s[1] } ) .orElse( new String[] { realClassName, realClassName } ); @@ -334,26 +333,34 @@ private String[] toClassMethodName( TestIdentifier testIdentifier ) boolean needsSpaceSeparator = isNotBlank( parentDisplay ) && !display.startsWith( "[" ); String methodDisplay = parentDisplay + ( needsSpaceSeparator ? " " : "" ) + display; - String simpleClassNames = COMMA_PATTERN.splitAsStream( methodSource.getMethodParameterTypes() ) - .map( s -> s.substring( 1 + s.lastIndexOf( '.' ) ) ) - .collect( joining( "," ) ); + boolean hasParameterizedParent = + collectAllTestIdentifiersInHierarchy( testIdentifier ) + .filter( identifier -> !identifier.getSource().isPresent() ) + .map( TestIdentifier::getLegacyReportingName ) + .anyMatch( legacyReportingName -> legacyReportingName.matches( "^\\[.+]$" ) ); - boolean hasParams = isNotBlank( methodSource.getMethodParameterTypes() ); + boolean parameterized = isNotBlank( methodSource.getMethodParameterTypes() ) || hasParameterizedParent; String methodName = methodSource.getMethodName(); String description = testIdentifier.getLegacyReportingName(); - String methodSign = hasParams ? methodName + '(' + simpleClassNames + ')' : methodName; boolean equalDescriptions = methodDisplay.equals( description ); boolean hasLegacyDescription = description.startsWith( methodName + '(' ); boolean hasDisplayName = !equalDescriptions || !hasLegacyDescription; - String methodDesc = equalDescriptions || !hasParams ? methodSign : description; + String methodDesc = parameterized ? description : methodName; String methodDisp = hasDisplayName ? methodDisplay : methodDesc; // The behavior of methods getLegacyReportingName() and getDisplayName(). - // test || legacy | display + // junit4 || legacy | display + // ==============||==========|========== + // normal || m | m + // param || m[0] | m[0] + // param+displ || m[displ] | m[displ] + + // junit5 || legacy | display // ==============||==========|========== // normal || m() | m() - // normal+displ || displ | displ - // parameterized || m()[1] | displ + // normal+displ || m() | displ + // param || m()[1] | [1] + // param+displ || m()[1] | displ return new String[] {source[0], source[1], methodDesc, methodDisp}; }