From 5f3d378d4ef29bc83e6c98ea2280ffe96760cea6 Mon Sep 17 00:00:00 2001 From: Hugo G Date: Thu, 26 Sep 2024 21:27:04 +1000 Subject: [PATCH 1/4] Sample code for Junit5's TestContext reporting bug --- .../resources/junit5-testtemplate-bug/pom.xml | 45 ++++++++++++++ .../src/test/java/pkg/FieldSettingTest.java | 62 +++++++++++++++++++ .../src/test/java/pkg/ParamsContextTest.java | 62 +++++++++++++++++++ 3 files changed, 169 insertions(+) create mode 100644 surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml create mode 100644 surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/FieldSettingTest.java create mode 100644 surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/ParamsContextTest.java diff --git a/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml b/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml new file mode 100644 index 0000000000..8ae915493b --- /dev/null +++ b/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml @@ -0,0 +1,45 @@ + + + 4.0.0 + + com.atlassian.oss + surefire-junit-testtemplate-retry-bug + 1.0-SNAPSHOT + + + 21 + 21 + UTF-8 + + + + + + org.junit + junit-bom + 5.11.1 + pom + import + + + + + + + org.junit.jupiter + junit-jupiter + test + + + + + + maven-surefire-plugin + 3.5.0 + + + + + diff --git a/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/FieldSettingTest.java b/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/FieldSettingTest.java new file mode 100644 index 0000000000..b17237b006 --- /dev/null +++ b/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/FieldSettingTest.java @@ -0,0 +1,62 @@ +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.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 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 "[%d] %s".formatted(invocationIndex, parameter); + } + + @Override + public List getAdditionalExtensions() { + return getBeforeEachCallbacks(parameter); + } + }; + } + + private List getBeforeEachCallbacks(int value) { + return List.of(((BeforeEachCallback) ctx -> + ((FieldSettingTest) ctx.getRequiredTestInstance()).setTestValue(value) + )); + } +} diff --git a/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/ParamsContextTest.java b/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/ParamsContextTest.java new file mode 100644 index 0000000000..aa2298779d --- /dev/null +++ b/surefire-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(Integer value) { + assertEquals(42, value); + } +} + +class ParamsContextProvider implements TestTemplateInvocationContextProvider { + + @Override + public boolean supportsTestTemplate(ExtensionContext context) { + return true; + } + + @Override + public Stream 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 "[%d] %s".formatted(invocationIndex, parameter); + } + + @Override + public List getAdditionalExtensions() { + return Collections.singletonList(new ParameterResolver() { + @Override + public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) { + return parameterContext.getParameter().getType().equals(Integer.class); + } + + @Override + public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) { + return parameter; + } + }); + } + }; + } +} From 01faf7f149fd606230b997d4ec538f69ce428cb3 Mon Sep 17 00:00:00 2001 From: Hugo G Date: Thu, 3 Oct 2024 14:22:01 +1000 Subject: [PATCH 2/4] Parametrised pom and failing test case --- .../surefire/its/JUnitPlatformEnginesIT.java | 29 +++++++ .../resources/junit5-testtemplate-bug/pom.xml | 76 ++++++++++--------- 2 files changed, 69 insertions(+), 36 deletions(-) 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 index ba79ccb9ff..81d36c1d65 100644 --- 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,35 @@ public void testJupiterEngineWithFailureInTestTemplateProvider() { .assertContainsText("Encountered failure in TestTemplate provideTestTemplateInvocationContexts()"); } + @Test + public void testJupiterEngineWithTestTemplateNotClassifiedAsFlake() { + // Similar example, but using parameterized test (which is a test template under the hood) + //String testToRun = "ParamsContextTest"; + String testToRun = "FieldSettingTest"; + unpack("junit5-testtemplate-bug", "-" + jupiter) + .setTestToRun(testToRun) + .sysProp("junit5.version", jupiter) + //.debugSurefireFork() + .maven() + .withFailure() + .executeTest() + .verifyTextInLog("AssertionFailedError") + .assertTestSuiteResults(2, 0, 1, 0, 0); + + unpack("junit5-testtemplate-bug", "-" + jupiter) + .debugLogging() + .setTestToRun(testToRun) + .sysProp("junit5.version", jupiter) + // The tests are failing deterministically, so rerunning them should not change the result + .sysProp("surefire.rerunFailingTestsCount", "1") + //.debugSurefireFork() + .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 index 8ae915493b..4ff2fe85f2 100644 --- a/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml +++ b/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml @@ -2,44 +2,48 @@ - 4.0.0 + 4.0.0 - com.atlassian.oss - surefire-junit-testtemplate-retry-bug - 1.0-SNAPSHOT + org.apache.maven.plugins.surefire + surefire-junit-testtemplate-retry-bug + 1.0-SNAPSHOT + Test for JUnit 5 TestTemplate with retries - - 21 - 21 - UTF-8 - + + ${java.specification.version} + ${java.specification.version} + UTF-8 + - - - - org.junit - junit-bom - 5.11.1 - pom - import - - - - - - - org.junit.jupiter - junit-jupiter - test - - - - - - maven-surefire-plugin - 3.5.0 - - - + + + org.junit.jupiter + junit-jupiter-engine + ${junit5.version} + test + + + org.junit.jupiter + junit-jupiter-params + ${junit5.version} + test + + + + + + maven-compiler-plugin + 3.8.1 + + UTF-8 + + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + + From 8aaf0a6def069e423b19a3b7b9342a17be52202b Mon Sep 17 00:00:00 2001 From: Hugo G Date: Thu, 3 Oct 2024 17:00:50 +1000 Subject: [PATCH 3/4] Fix for test template retries. Also added test for parametrised tests --- .../surefire/its/JUnitPlatformEnginesIT.java | 54 +++++++++++-------- .../junitplatform/RunListenerAdapter.java | 4 +- 2 files changed, 35 insertions(+), 23 deletions(-) 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 index 81d36c1d65..255aea2918 100644 --- 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 @@ -298,31 +298,41 @@ public void testJupiterEngineWithFailureInTestTemplateProvider() { @Test public void testJupiterEngineWithTestTemplateNotClassifiedAsFlake() { - // Similar example, but using parameterized test (which is a test template under the hood) - //String testToRun = "ParamsContextTest"; - String testToRun = "FieldSettingTest"; unpack("junit5-testtemplate-bug", "-" + jupiter) - .setTestToRun(testToRun) - .sysProp("junit5.version", jupiter) - //.debugSurefireFork() - .maven() - .withFailure() - .executeTest() - .verifyTextInLog("AssertionFailedError") - .assertTestSuiteResults(2, 0, 1, 0, 0); + .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(testToRun) - .sysProp("junit5.version", jupiter) - // The tests are failing deterministically, so rerunning them should not change the result - .sysProp("surefire.rerunFailingTestsCount", "1") - //.debugSurefireFork() - .maven() - .withFailure() - .executeTest() - .verifyTextInLog("AssertionFailedError") - .assertTestSuiteResults(2, 0, 1, 0, 0); + .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 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 8c274fd1c9..7ae8cc79d3 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 @@ -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); From 4f52c2d488e9fd35604c6c6a590ea9f7568a0bc1 Mon Sep 17 00:00:00 2001 From: Hugo G Date: Wed, 9 Oct 2024 13:59:20 +1100 Subject: [PATCH 4/4] Fix for compilation on Java 8 and older Junit versions --- .../src/test/resources/junit5-testtemplate-bug/pom.xml | 6 ++++++ .../src/test/java/pkg/FieldSettingTest.java | 7 ++++--- .../src/test/java/pkg/ParamsContextTest.java | 6 +++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml b/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml index 4ff2fe85f2..335ba72b53 100644 --- a/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml +++ b/surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml @@ -22,6 +22,12 @@ ${junit5.version} test + + org.junit.jupiter + junit-jupiter-api + ${junit5.version} + test + org.junit.jupiter junit-jupiter-params diff --git a/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/FieldSettingTest.java b/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/FieldSettingTest.java index b17237b006..61f5fa5c52 100644 --- a/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/FieldSettingTest.java +++ b/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/FieldSettingTest.java @@ -8,6 +8,7 @@ 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; @@ -44,7 +45,7 @@ private TestTemplateInvocationContext context(int parameter) { return new TestTemplateInvocationContext() { @Override public String getDisplayName(int invocationIndex) { - return "[%d] %s".formatted(invocationIndex, parameter); + return String.format("[%d] %s", invocationIndex, parameter); } @Override @@ -55,8 +56,8 @@ public List getAdditionalExtensions() { } private List getBeforeEachCallbacks(int value) { - return List.of(((BeforeEachCallback) ctx -> + return Arrays.asList((BeforeEachCallback) ctx -> ((FieldSettingTest) ctx.getRequiredTestInstance()).setTestValue(value) - )); + ); } } diff --git a/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/ParamsContextTest.java b/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/ParamsContextTest.java index aa2298779d..4022e2cbb1 100644 --- a/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/ParamsContextTest.java +++ b/surefire-its/src/test/resources/junit5-testtemplate-bug/src/test/java/pkg/ParamsContextTest.java @@ -19,7 +19,7 @@ public class ParamsContextTest { @TestTemplate @ExtendWith(ParamsContextProvider.class) - void testTemplatePartiallyFails(Integer value) { + void testTemplatePartiallyFails(int value) { assertEquals(42, value); } } @@ -40,7 +40,7 @@ private TestTemplateInvocationContext invocationContext(int parameter) { return new TestTemplateInvocationContext() { @Override public String getDisplayName(int invocationIndex) { - return "[%d] %s".formatted(invocationIndex, parameter); + return String.format("[%d] %s", invocationIndex, parameter); } @Override @@ -48,7 +48,7 @@ public List getAdditionalExtensions() { return Collections.singletonList(new ParameterResolver() { @Override public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) { - return parameterContext.getParameter().getType().equals(Integer.class); + return true; } @Override