Improve error message for non-static external @MethodSource factory methods

[sbrannen]

Overview

After reviewing #3263, I realized that the error message is also cryptic if an external factory method is non-static when the test class is configured to run with test instance per-class lifecycle semantics.

For the record, prior to #3263, the error message was also cryptic if an external factory method was non-static when the test class was configured to run with test instance per-method lifecycle semantics.

Example

package example;

import java.util.stream.Stream;

import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

@TestInstance(Lifecycle.PER_CLASS)
class MethodSourceTests {

	@ParameterizedTest
	@MethodSource("example.Utils#fruits")
	void test(String fruit) {
		System.err.println(fruit);
	}

}

class Utils {
	Stream<String> fruits() {
		return Stream.of("apple", "banana");
	}
}

The test method above fails with:

java.lang.IllegalArgumentException: object is not an instance of declaring class
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:732)
	at org.junit.jupiter.engine.execution.DefaultExecutableInvoker.invoke(DefaultExecutableInvoker.java:55)
	at org.junit.jupiter.params.provider.MethodArgumentsProvider.lambda$2(MethodArgumentsProvider.java:55)

Definition of Done

• Improve error message for non-static external @MethodSource factory methods
• Change is covered by automated tests including corner cases, errors, and exception handling
• Change is documented in the Release Notes

[sbrannen]

The following appears to be a valid solution (as a replacement for the current version of validateFactoryMethod()); however, I have yet to implement appropriate tests for all use cases.

	private static Method validateFactoryMethod(ExtensionContext context, Method factoryMethod, Object testInstance) {
		if (!factoryMethod.getDeclaringClass().isInstance(testInstance)) {
			Preconditions.condition(ReflectionUtils.isStatic(factoryMethod),
				() -> format("Method '%s' must be static: local factory methods must be static unless "
						+ "the test class is annotated with @TestInstance(Lifecycle.PER_CLASS); external "
						+ "factory methods must always be static.",
					factoryMethod.toGenericString()));
		}
		return factoryMethod;
	}

[sbrannen]

Reopening to update the Release Notes.

[sbrannen]

@BassemElMasry, you might find the solution to this one (and the subsequent simplification in 6cb0858) interesting since it's related to your work in #3263.