Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SUREFIRE-2276] - Fix for retries of JUnit TestTemplate tests #788

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
55 changes: 55 additions & 0 deletions surefire-its/src/test/resources/junit5-testtemplate-bug/pom.xml
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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)
);
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
});
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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+]$");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work only for numeric parameters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the good question!

A test template function gets invoked within a loop (stream to be precise), once for each invocation context. This legacy name contains the index of the invocation and is unrelated to any params the method actually receives, and even unrelated to the display name we set in the invocation context.

I just also double checked by changing the integers used in the test fixtures to strings, and the tests pass just fine.


boolean parameterized = isNotBlank(methodSource.getMethodParameterTypes()) || hasParameterizedParent;
boolean parameterized = isParameterized || hasParameterizedParent || isTestTemplate;
String methodName = methodSource.getMethodName();
String description = testIdentifier.getLegacyReportingName();
boolean equalDescriptions = methodDisplay.equals(description);
Expand Down