Improving error message for non-static @MethodSource factory methods issue: #3215

Author: BassemElMasry

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;
@@ -46,12 +47,26 @@ protected Stream<? extends Arguments> provideArguments(ExtensionContext context,
 		// @formatter:off
 		return stream(methodNames)
 				.map(factoryMethodName -> getFactoryMethod(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),
+				factoryMethod + " method must not be static");
+		}
+		return factoryMethod;
+	}
+
+	private boolean isPerMethodLifecycle(ExtensionContext context) {
+		return context.getTestInstanceLifecycle().orElse(
+			TestInstance.Lifecycle.PER_CLASS) == TestInstance.Lifecycle.PER_METHOD;
+	}
+
 	private Method getFactoryMethod(Class<?> testClass, Method testMethod, String factoryMethodName) {
 		if (!StringUtils.isBlank(factoryMethodName)) {
 			if (looksLikeAFullyQualifiedMethodName(factoryMethodName)) {

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

@@ -33,6 +33,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;
@@ -198,7 +199,7 @@ void throwsExceptionWhenNonStaticFactoryMethodIsReferencedAndStaticIsRequired()
 		var exception = assertThrows(JUnitException.class,
 			() -> provideArguments(NonStaticTestCase.class, null, false, "nonStaticStringStreamProvider").toArray());
 
-		assertThat(exception).hasMessageContaining("Cannot invoke non-static method");
+		assertThat(exception).hasMessageContaining("method must not be static");
 	}
 
 	@Test
@@ -609,6 +610,9 @@ private Stream<Object[]> provideArguments(Class<?> testClass, Method testMethod,
 		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);
@@ -796,6 +800,13 @@ Stream<String> nonStaticStringStreamProvider() {
 		}
 	}
 
+	@TestInstance(TestInstance.Lifecycle.PER_METHOD)
+	static class NonStaticTestCaseForPerMethodLifecycle {
+		Stream<String> nonStaticStringStreamProvider() {
+			return Stream.of("foo", "bar");
+		}
+	}
+
 	static class ExternalFactoryMethods {
 
 		static Stream<String> stringsProvider() {