Skip to content

Commit 0aae9a9

Browse files
Improve error message for non-static @MethodSource factory methods
Resolves #3215. Co-authored-by: Marc Philipp <[email protected]>
1 parent 4d69f8c commit 0aae9a9

File tree

3 files changed

+23
-1
lines changed

3 files changed

+23
-1
lines changed

documentation/src/docs/asciidoc/release-notes/release-notes-5.10.0-M1.adoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ default.
8686
temporary directories. See the
8787
<<../user-guide/index.adoc#writing-tests-built-in-extensions-TempDirectory, User Guide>>
8888
for details.
89+
* Improve error message for non-static `@MethodSource` factory methods
8990

9091

9192
[[release-notes-5.10.0-M1-junit-vintage]]

junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import org.junit.jupiter.api.Test;
2626
import org.junit.jupiter.api.TestFactory;
27+
import org.junit.jupiter.api.TestInstance;
2728
import org.junit.jupiter.api.TestTemplate;
2829
import org.junit.jupiter.api.extension.ExtensionContext;
2930
import org.junit.platform.commons.JUnitException;
@@ -50,12 +51,27 @@ protected Stream<? extends Arguments> provideArguments(ExtensionContext context,
5051
// @formatter:off
5152
return stream(methodNames)
5253
.map(factoryMethodName -> findFactoryMethod(testClass, testMethod, factoryMethodName))
54+
.map(factoryMethod -> validateStaticFactoryMethod(context, factoryMethod))
5355
.map(factoryMethod -> context.getExecutableInvoker().invoke(factoryMethod, testInstance))
5456
.flatMap(CollectionUtils::toStream)
5557
.map(MethodArgumentsProvider::toArguments);
5658
// @formatter:on
5759
}
5860

61+
private Method validateStaticFactoryMethod(ExtensionContext context, Method factoryMethod) {
62+
if (isPerMethodLifecycle(context)) {
63+
Preconditions.condition(ReflectionUtils.isStatic(factoryMethod), () -> String.format(
64+
"method '%s' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).",
65+
factoryMethod.toGenericString()));
66+
}
67+
return factoryMethod;
68+
}
69+
70+
private boolean isPerMethodLifecycle(ExtensionContext context) {
71+
return context.getTestInstanceLifecycle().orElse(
72+
TestInstance.Lifecycle.PER_CLASS) == TestInstance.Lifecycle.PER_METHOD;
73+
}
74+
5975
private static Method findFactoryMethod(Class<?> testClass, Method testMethod, String factoryMethodName) {
6076
String originalFactoryMethodName = factoryMethodName;
6177

junit-jupiter-params/src/test/java/org/junit/jupiter/params/provider/MethodArgumentsProviderTests.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.junit.jupiter.api.BeforeEach;
3333
import org.junit.jupiter.api.Nested;
3434
import org.junit.jupiter.api.Test;
35+
import org.junit.jupiter.api.TestInstance;
3536
import org.junit.jupiter.api.extension.ExtensionContext;
3637
import org.junit.jupiter.api.extension.ParameterContext;
3738
import org.junit.jupiter.api.extension.ParameterResolver;
@@ -177,7 +178,8 @@ void throwsExceptionWhenNonStaticFactoryMethodIsReferencedAndStaticIsRequired()
177178
var exception = assertThrows(PreconditionViolationException.class,
178179
() -> provideArguments(NonStaticTestCase.class, null, false, "nonStaticStringStreamProvider").toArray());
179180

180-
assertThat(exception).hasMessageContaining("Cannot invoke non-static method");
181+
assertThat(exception).hasMessageContaining(
182+
"must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS)");
181183
}
182184

183185
@Test
@@ -713,6 +715,9 @@ public void dummyMethod() {
713715
var testInstance = allowNonStaticMethod ? ReflectionUtils.newInstance(testClass) : null;
714716
when(extensionContext.getTestInstance()).thenReturn(Optional.ofNullable(testInstance));
715717

718+
var lifeCycle = allowNonStaticMethod ? TestInstance.Lifecycle.PER_CLASS : TestInstance.Lifecycle.PER_METHOD;
719+
when(extensionContext.getTestInstanceLifecycle()).thenReturn(Optional.of(lifeCycle));
720+
716721
var provider = new MethodArgumentsProvider();
717722
provider.accept(methodSource);
718723
return provider.provideArguments(extensionContext).map(Arguments::get);

0 commit comments

Comments
 (0)