diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.0-M1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.0-M1.adoc index a20ddfe9b82b..8529843fe89d 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.0-M1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.0-M1.adoc @@ -86,6 +86,7 @@ default. temporary directories. See the <<../user-guide/index.adoc#writing-tests-built-in-extensions-TempDirectory, User Guide>> for details. +* Improve error message for non-static `@MethodSource` factory methods [[release-notes-5.10.0-M1-junit-vintage]] diff --git a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java index 0336401b3886..6815c9cf1b71 100644 --- a/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java +++ b/junit-jupiter-params/src/main/java/org/junit/jupiter/params/provider/MethodArgumentsProvider.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestFactory; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.platform.commons.JUnitException; @@ -50,12 +51,27 @@ protected Stream provideArguments(ExtensionContext context, // @formatter:off return stream(methodNames) .map(factoryMethodName -> findFactoryMethod(testClass, testMethod, factoryMethodName)) + .map(factoryMethod -> validateStaticFactoryMethod(context, factoryMethod)) .map(factoryMethod -> context.getExecutableInvoker().invoke(factoryMethod, testInstance)) .flatMap(CollectionUtils::toStream) .map(MethodArgumentsProvider::toArguments); // @formatter:on } + private Method validateStaticFactoryMethod(ExtensionContext context, Method factoryMethod) { + if (isPerMethodLifecycle(context)) { + Preconditions.condition(ReflectionUtils.isStatic(factoryMethod), () -> String.format( + "method '%s' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).", + factoryMethod.toGenericString())); + } + return factoryMethod; + } + + private boolean isPerMethodLifecycle(ExtensionContext context) { + return context.getTestInstanceLifecycle().orElse( + TestInstance.Lifecycle.PER_CLASS) == TestInstance.Lifecycle.PER_METHOD; + } + private static Method findFactoryMethod(Class testClass, Method testMethod, String factoryMethodName) { String originalFactoryMethodName = factoryMethodName; diff --git a/junit-jupiter-params/src/test/java/org/junit/jupiter/params/provider/MethodArgumentsProviderTests.java b/junit-jupiter-params/src/test/java/org/junit/jupiter/params/provider/MethodArgumentsProviderTests.java index 1bd49479bb06..24c144f21c0f 100644 --- a/junit-jupiter-params/src/test/java/org/junit/jupiter/params/provider/MethodArgumentsProviderTests.java +++ b/junit-jupiter-params/src/test/java/org/junit/jupiter/params/provider/MethodArgumentsProviderTests.java @@ -32,6 +32,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.extension.ParameterContext; import org.junit.jupiter.api.extension.ParameterResolver; @@ -177,7 +178,8 @@ void throwsExceptionWhenNonStaticFactoryMethodIsReferencedAndStaticIsRequired() var exception = assertThrows(PreconditionViolationException.class, () -> provideArguments(NonStaticTestCase.class, null, false, "nonStaticStringStreamProvider").toArray()); - assertThat(exception).hasMessageContaining("Cannot invoke non-static method"); + assertThat(exception).hasMessageContaining( + "must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS)"); } @Test @@ -713,6 +715,9 @@ public void dummyMethod() { var testInstance = allowNonStaticMethod ? ReflectionUtils.newInstance(testClass) : null; when(extensionContext.getTestInstance()).thenReturn(Optional.ofNullable(testInstance)); + var lifeCycle = allowNonStaticMethod ? TestInstance.Lifecycle.PER_CLASS : TestInstance.Lifecycle.PER_METHOD; + when(extensionContext.getTestInstanceLifecycle()).thenReturn(Optional.of(lifeCycle)); + var provider = new MethodArgumentsProvider(); provider.accept(methodSource); return provider.provideArguments(extensionContext).map(Arguments::get);