[SUREFIRE-2276] JUnit5's TestTemplate failures treated as flakes with…

… retries This closes #788
apache · Oct 29, 2024 · e1f94a0 · e1f94a0
1 parent d24adb4
commit e1f94a0
Show file tree

Hide file tree

Showing 5 changed files with 222 additions and 1 deletion.
diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/JUnitPlatformEnginesIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/JUnitPlatformEnginesIT.java
@@ -296,6 +296,45 @@ public void testJupiterEngineWithFailureInTestTemplateProvider() {
                 .assertContainsText("Encountered failure in TestTemplate provideTestTemplateInvocationContexts()");
     }
 
+    @Test
+    public void testJupiterEngineWithTestTemplateNotClassifiedAsFlake() {
+        unpack("junit5-testtemplate-bug", "-" + jupiter)
+                .setTestToRun("FieldSettingTest")
+                .sysProp("junit5.version", jupiter)
+                .maven()
+                .withFailure()
+                .executeTest()
+                .verifyTextInLog("AssertionFailedError")
+                .assertTestSuiteResults(2, 0, 1, 0, 0);
+
+        unpack("junit5-testtemplate-bug", "-" + jupiter)
+                .debugLogging()
+                .setTestToRun("FieldSettingTest")
+                .sysProp("junit5.version", jupiter)
+                // The tests are failing deterministically, so rerunning them should not change the result
+                .sysProp("surefire.rerunFailingTestsCount", "1")
+                .maven()
+                .withFailure()
+                .executeTest()
+                .verifyTextInLog("AssertionFailedError")
+                .assertTestSuiteResults(2, 0, 1, 0, 0);
+    }
+
+    @Test
+    public void testJupiterEngineWithParameterizedTestsNotClassifiedAsFlake() {
+        unpack("junit5-testtemplate-bug", "-" + jupiter)
+                .debugLogging()
+                .setTestToRun("ParamsContextTest")
+                .sysProp("junit5.version", jupiter)
+                // The tests are failing deterministically, so rerunning them should not change the result
+                .sysProp("surefire.rerunFailingTestsCount", "1")
+                .maven()
+                .withFailure()
+                .executeTest()
+                .verifyTextInLog("AssertionFailedError")
+                .assertTestSuiteResults(2, 0, 1, 0, 0);
+    }
+
     @Test
     public void testJupiterEngineWithAssertionsFailNoParameters() {
         // `Assertions.fail()` not supported until 5.2.0

diff --git a/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml b/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml
@@ -0,0 +1,55 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>org.apache.maven.plugins.surefire</groupId>
+  <artifactId>surefire-junit-testtemplate-retry-bug</artifactId>
+  <version>1.0-SNAPSHOT</version>
+  <name>Test for JUnit 5 TestTemplate with retries</name>
+
+  <properties>
+    <maven.compiler.source>${java.specification.version}</maven.compiler.source>
+    <maven.compiler.target>${java.specification.version}</maven.compiler.target>
+    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+  </properties>
+
+  <dependencies>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-engine</artifactId>
+      <version>${junit5.version}</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-api</artifactId>
+      <version>${junit5.version}</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-params</artifactId>
+      <version>${junit5.version}</version>
+      <scope>test</scope>
+    </dependency>
+  </dependencies>
+  <build>
+    <plugins>
+      <plugin>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <version>3.8.1</version>
+        <configuration>
+          <encoding>UTF-8</encoding>
+        </configuration>
+      </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <version>${surefire.version}</version>
+      </plugin>
+    </plugins>
+  </build>
+
+</project>
diff --git a/...re-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/FieldSettingTest.java b/...re-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/FieldSettingTest.java
@@ -0,0 +1,63 @@
+package pkg;
+
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.extension.Extension;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
+import org.junit.jupiter.api.extension.TestTemplateInvocationContextProvider;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class FieldSettingTest {
+    private int testValue = 42;
+
+    // We're calling this in the provider underneath
+    public void setTestValue(int testValue) {
+        this.testValue = testValue;
+    }
+
+    @TestTemplate
+    @ExtendWith(FieldSettingContextProvider.class)
+    public void testTemplatePartiallyFails() {
+        assertEquals(42, testValue);
+    }
+}
+
+
+class FieldSettingContextProvider implements TestTemplateInvocationContextProvider {
+    @Override
+    public boolean supportsTestTemplate(ExtensionContext extensionContext) {
+        return true;
+    }
+
+    @Override
+    public Stream<TestTemplateInvocationContext> provideTestTemplateInvocationContexts(ExtensionContext extensionContext) {
+        return Stream.of(context(0), context(42));
+    }
+
+    private TestTemplateInvocationContext context(int parameter) {
+        return new TestTemplateInvocationContext() {
+            @Override
+            public String getDisplayName(int invocationIndex) {
+                return String.format("[%d] %s", invocationIndex, parameter);
+            }
+
+            @Override
+            public List<Extension> getAdditionalExtensions() {
+                return getBeforeEachCallbacks(parameter);
+            }
+        };
+    }
+
+    private List<Extension> getBeforeEachCallbacks(int value) {
+        return Arrays.asList((BeforeEachCallback) ctx ->
+                ((FieldSettingTest) ctx.getRequiredTestInstance()).setTestValue(value)
+        );
+    }
+}
diff --git a/...e-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/ParamsContextTest.java b/...e-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/ParamsContextTest.java
@@ -0,0 +1,62 @@
+package pkg;
+
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.extension.Extension;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ParameterContext;
+import org.junit.jupiter.api.extension.ParameterResolver;
+import org.junit.jupiter.api.extension.TestTemplateInvocationContext;
+import org.junit.jupiter.api.extension.TestTemplateInvocationContextProvider;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class ParamsContextTest {
+
+    @TestTemplate
+    @ExtendWith(ParamsContextProvider.class)
+    void testTemplatePartiallyFails(int value) {
+        assertEquals(42, value);
+    }
+}
+
+class ParamsContextProvider implements TestTemplateInvocationContextProvider {
+
+    @Override
+    public boolean supportsTestTemplate(ExtensionContext context) {
+        return true;
+    }
+
+    @Override
+    public Stream<TestTemplateInvocationContext> provideTestTemplateInvocationContexts(ExtensionContext context) {
+        return Stream.of(invocationContext(0), invocationContext(42));
+    }
+
+    private TestTemplateInvocationContext invocationContext(int parameter) {
+        return new TestTemplateInvocationContext() {
+            @Override
+            public String getDisplayName(int invocationIndex) {
+                return String.format("[%d] %s", invocationIndex, parameter);
+            }
+
+            @Override
+            public List<Extension> getAdditionalExtensions() {
+                return Collections.singletonList(new ParameterResolver() {
+                    @Override
+                    public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
+                        return true;
+                    }
+
+                    @Override
+                    public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
+                        return parameter;
+                    }
+                });
+            }
+        };
+    }
+}
diff --git a/...it-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java b/...it-platform/src/main/java/org/apache/maven/surefire/junitplatform/RunListenerAdapter.java
@@ -317,12 +317,14 @@ private String[] toClassMethodName(TestIdentifier testIdentifier) {
             boolean needsSpaceSeparator = isNotBlank(parentDisplay) && !display.startsWith("[");
             String methodDisplay = parentDisplay + (needsSpaceSeparator ? " " : "") + display;
 
+            boolean isParameterized = isNotBlank(methodSource.getMethodParameterTypes());
             boolean hasParameterizedParent = collectAllTestIdentifiersInHierarchy(testIdentifier)
                     .filter(identifier -> !identifier.getSource().isPresent())
                     .map(TestIdentifier::getLegacyReportingName)
                     .anyMatch(legacyReportingName -> legacyReportingName.matches("^\\[.+]$"));
+            boolean isTestTemplate = testIdentifier.getLegacyReportingName().matches("^.*\\[\\d+]$");
 
-            boolean parameterized = isNotBlank(methodSource.getMethodParameterTypes()) || hasParameterizedParent;
+            boolean parameterized = isParameterized || hasParameterizedParent || isTestTemplate;
             String methodName = methodSource.getMethodName();
             String description = testIdentifier.getLegacyReportingName();
             boolean equalDescriptions = methodDisplay.equals(description);