Stop using a conventional suffix for TestBean factory methods

This commit changes how factory method for `@TestBean` usage is
discovered. Previously the field name or bean name suffixed with
'TestOverride' was used. It sounds more natural to just use the
field name or bean name, leaving cases where a suffix is required
to explicitly providing the method name.

As part of this change, the exception messages have been revisited as
it's less since the method name candidates have the exact same name
as the field or bean name. A `()` is added to make it more clear the
name is for a method.

Closes gh-32940

Author: snicoll

framework-docs/modules/ROOT/pages/testing/annotations/integration-spring/annotation-testbean.adoc

@@ -5,19 +5,19 @@
 `ApplicationContext` with an instance provided by a conventionally named static factory
 method.
 
-By default, the associated factory method name is derived from the annotated field's name,
-but the annotation allows for a specific method name to be provided.
+The associated factory method name is derived from the annotated field's name, or bean
+name if specified. A `static` method with no argument that returns a type compatible
+with the type of the bean to override is expected. To make things more explicit, or if
+you'd rather use a different name, the annotation allows for a specific method name to
+be provided.
 
-The `@TestBean` annotation uses the `REPLACE_DEFINITION`
-xref:testing/testcontext-framework/bean-overriding.adoc#testcontext-bean-overriding-custom[strategy for test bean overriding].
 
 By default, the annotated field's type is used to search for candidate definitions to override.
 In that case it is required that exactly one definition matches, but note that `@Qualifier`
 annotations are also taken into account for the purpose of matching.
 Users can also make things entirely explicit by specifying a bean `name` in the annotation.
 
-The following example shows how to fully configure the `@TestBean` annotation, with
-explicit values equivalent to the defaults:
+The following example shows how to use the default behavior of the `@TestBean` annotation:
 
 [tabs]
 ======
@@ -26,17 +26,41 @@ Java::
 [source,java,indent=0,subs="verbatim,quotes",role="primary"]
 ----
 	class OverrideBeanTests {
-		@TestBean(name = "service", methodName = "serviceTestOverride")  // <1>
+		@TestBean  // <1>
 		private CustomService service;
 
 		// test case body...
 
-		private static CustomService serviceTestOverride() { // <2>
+		private static CustomService service() { // <2>
 			return new MyFakeCustomService();
 		}
 	}
 ----
-<1> Mark a field for bean overriding in this test class.
+<1> Mark a field for overriding of the bean with type `CustomService`.
+<2> The result of this static method will be used as the instance and injected into the field.
+======
+
+
+The following example shows how to fully configure the `@TestBean` annotation:
+
+[tabs]
+======
+Java::
++
+[source,java,indent=0,subs="verbatim,quotes",role="primary"]
+----
+	class OverrideBeanTests {
+		@TestBean(name = "service", methodName = "createCustomService")  // <1>
+		private CustomService service;
+
+		// test case body...
+
+		private static CustomService createCustomService() { // <2>
+			return new MyFakeCustomService();
+		}
+	}
+----
+<1> Mark a field for overriding of the bean with name `service`.
 <2> The result of this static method will be used as the instance and injected into the field.
 ======
 

spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBean.java

@@ -43,9 +43,9 @@
  * <ul>
  * <li>If the {@link #methodName()} is specified, look for a static method with
  * that name.</li>
- * <li>If a method name is not specified, look for exactly one static method named
- * with a suffix equal to {@value #CONVENTION_SUFFIX} and starting with either the
- * name of the annotated field or the name of the bean (if specified).</li>
+ * <li>If a method name is not specified, look for exactly one static method
+ * named with either the name of the annotated field or the name of the bean
+ * (if specified).</li>
  * </ul>
  *
  * <p>Consider the following example.
@@ -58,24 +58,24 @@
  *
  *     // Tests
  *
- *     private static CustomerRepository repositoryTestOverride() {
+ *     private static CustomerRepository repository() {
  *         return new TestCustomerRepository();
  *     }
  * }</code></pre>
  *
  * <p>In the example above, the {@code repository} bean is replaced by the
- * instance generated by the {@code repositoryTestOverride()} method. Not only
- * is the overridden instance injected into the {@code repository} field, but it
- * is also replaced in the {@code BeanFactory} so that other injection points
- * for that bean use the overridden bean instance.
+ * instance generated by the {@code repository()} method. Not only is the
+ * overridden instance injected into the {@code repository} field, but it is
+ * also replaced in the {@code BeanFactory} so that other injection points for
+ * that bean use the overridden bean instance.
  *
  * <p>To make things more explicit, the bean and method names can be set,
  * as shown in the following example.
  *
  * <pre><code>
  * class CustomerServiceTests {
  *
- *     &#064;TestBean(name = "repository", methodName = "createTestCustomerRepository")
+ *     &#064;TestBean(name = "customerRepository", methodName = "createTestCustomerRepository")
  *     private CustomerRepository repository;
  *
  *     // Tests
@@ -97,14 +97,6 @@
 @BeanOverride(TestBeanOverrideProcessor.class)
 public @interface TestBean {
 
-	/**
-	 * Required suffix for the name of a factory method that overrides a bean
-	 * instance when the factory method is detected by convention.
-	 * @see #methodName()
-	 */
-	String CONVENTION_SUFFIX = "TestOverride";
-
-
 	/**
 	 * Alias for {@link #name()}.
 	 * <p>Intended to be used when no other attributes are needed &mdash; for
@@ -130,8 +122,7 @@
 	 * also considered. Similarly, in case the test class inherits from a base
 	 * class the whole class hierarchy is considered.
 	 * <p>If left unspecified, the name of the factory method will be detected
-	 * based on convention.
-	 * @see #CONVENTION_SUFFIX
+	 * based either on the name of the annotated field or the name of the bean.
 	 */
 	String methodName() default "";
 

spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessor.java

@@ -21,9 +21,11 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.springframework.core.MethodIntrospector;
 import org.springframework.core.ResolvableType;
@@ -41,6 +43,7 @@
  *
  * @author Simon Baslé
  * @author Sam Brannen
+ * @author Stephane Nicoll
  * @since 6.2
  */
 class TestBeanOverrideProcessor implements BeanOverrideProcessor {
@@ -58,14 +61,14 @@ public TestBeanOverrideMetadata createMetadata(Annotation overrideAnnotation, Cl
 			overrideMethod = findTestBeanFactoryMethod(testClass, field.getType(), methodName);
 		}
 		else {
-			// Otherwise, search for candidate factory methods using the convention
-			// suffix and the field name or explicit bean name (if any).
+			// Otherwise, search for candidate factory methods the field name
+			// or explicit bean name (if any).
 			List<String> candidateMethodNames = new ArrayList<>();
-			candidateMethodNames.add(field.getName() + TestBean.CONVENTION_SUFFIX);
+			candidateMethodNames.add(field.getName());
 
 			String beanName = testBeanAnnotation.name();
 			if (StringUtils.hasText(beanName)) {
-				candidateMethodNames.add(beanName + TestBean.CONVENTION_SUFFIX);
+				candidateMethodNames.add(beanName);
 			}
 			overrideMethod = findTestBeanFactoryMethod(testClass, field.getType(), candidateMethodNames);
 		}
@@ -75,7 +78,7 @@ public TestBeanOverrideMetadata createMetadata(Annotation overrideAnnotation, Cl
 
 	/**
 	 * Find a test bean factory {@link Method} for the given {@link Class}.
-	 * <p>Delegates to {@link #findTestBeanFactoryMethod(Class, Class, List)}.
+	 * <p>Delegates to {@link #findTestBeanFactoryMethod(Class, Class, Collection)}.
 	 */
 	Method findTestBeanFactoryMethod(Class<?> clazz, Class<?> methodReturnType, String... methodNames) {
 		return findTestBeanFactoryMethod(clazz, methodReturnType, List.of(methodNames));
@@ -104,7 +107,7 @@ Method findTestBeanFactoryMethod(Class<?> clazz, Class<?> methodReturnType, Stri
 	 * @throws IllegalStateException if a matching factory method cannot
 	 * be found or multiple methods match
 	 */
-	Method findTestBeanFactoryMethod(Class<?> clazz, Class<?> methodReturnType, List<String> methodNames) {
+	Method findTestBeanFactoryMethod(Class<?> clazz, Class<?> methodReturnType, Collection<String> methodNames) {
 		Assert.notEmpty(methodNames, "At least one candidate method name is required");
 		Set<String> supportedNames = new LinkedHashSet<>(methodNames);
 		MethodFilter methodFilter = method -> (Modifier.isStatic(method.getModifiers()) &&
@@ -113,16 +116,16 @@ Method findTestBeanFactoryMethod(Class<?> clazz, Class<?> methodReturnType, List
 
 		Set<Method> methods = findMethods(clazz, methodFilter);
 
-		Assert.state(!methods.isEmpty(), () -> """
-				Failed to find a static test bean factory method in %s with return type %s \
-				whose name matches one of the supported candidates %s""".formatted(
-				clazz.getName(), methodReturnType.getName(), supportedNames));
+		String methodNamesDescription = supportedNames.stream()
+				.map(name -> name + "()").collect(Collectors.joining(" or "));
+		Assert.state(!methods.isEmpty(), () ->
+				"No static method found named %s in %s with return type %s".formatted(
+						methodNamesDescription, clazz.getName(), methodReturnType.getName()));
 
 		long uniqueMethodNameCount = methods.stream().map(Method::getName).distinct().count();
-		Assert.state(uniqueMethodNameCount == 1, () -> """
-				Found %d competing static test bean factory methods in %s with return type %s \
-				whose name matches one of the supported candidates %s""".formatted(
-				uniqueMethodNameCount, clazz.getName(), methodReturnType.getName(), supportedNames));
+		Assert.state(uniqueMethodNameCount == 1, () ->
+				"Found %d competing static methods named %s in %s with return type %s".formatted(
+						uniqueMethodNameCount, methodNamesDescription, clazz.getName(), methodReturnType.getName()));
 
 		return methods.iterator().next();
 	}

spring-test/src/test/java/org/springframework/test/context/bean/override/convention/AbstractTestBeanIntegrationTestCase.java

@@ -32,11 +32,11 @@ abstract class AbstractTestBeanIntegrationTestCase {
 	@TestBean(name = "thirdBean")
 	Pojo anotherBean;
 
-	static Pojo otherBeanTestOverride() {
+	static Pojo otherBean() {
 		return new FakePojo("otherBean in superclass");
 	}
 
-	static Pojo thirdBeanTestOverride() {
+	static Pojo thirdBean() {
 		return new FakePojo("third in superclass");
 	}
 

spring-test/src/test/java/org/springframework/test/context/bean/override/convention/FailingTestBeanByTypeIntegrationTests.java

@@ -80,7 +80,7 @@ static class NoMatchingBeansTestCase {
 		void test() {
 		}
 
-		static ExampleService exampleTestOverride() {
+		static ExampleService example() {
 			return fail("unexpected override");
 		}
 
@@ -100,7 +100,7 @@ static class TooManyBeansTestCase {
 		void test() {
 		}
 
-		static ExampleService exampleTestOverride() {
+		static ExampleService example() {
 			return fail("unexpected override");
 		}
 

spring-test/src/test/java/org/springframework/test/context/bean/override/convention/FailingTestBeanInheritanceIntegrationTests.java

@@ -18,6 +18,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import org.springframework.test.context.bean.override.convention.AbstractTestBeanIntegrationTestCase.Pojo;
 import org.springframework.test.context.junit.EngineTestKitUtils;
 
 import static org.junit.platform.testkit.engine.EventConditions.finishedWithFailure;
@@ -40,10 +41,8 @@ void failsIfFieldInSupertypeButNoMethod() {
 		EngineTestKitUtils.executeTestsForClass(testClass).assertThatEvents().haveExactly(1,
 			finishedWithFailure(
 					instanceOf(IllegalStateException.class),
-					message("""
-						Failed to find a static test bean factory method in %s with return type %s \
-						whose name matches one of the supported candidates [someBeanTestOverride]"""
-							.formatted(testClass.getName(), AbstractTestBeanIntegrationTestCase.Pojo.class.getName()))));
+					message("No static method found named someBean() in %s with return type %s"
+							.formatted(FieldInSupertypeButNoMethodTestCase.class.getName(), Pojo.class.getName()))));
 	}
 
 	@Test
@@ -52,11 +51,8 @@ void failsIfMethod1InSupertypeAndMethod2InType() {
 		EngineTestKitUtils.executeTestsForClass(testClass).assertThatEvents().haveExactly(1,
 			finishedWithFailure(
 					instanceOf(IllegalStateException.class),
-					message("""
-						Found 2 competing static test bean factory methods in %s with return type %s \
-						whose name matches one of the supported candidates \
-						[anotherBeanTestOverride, thirdBeanTestOverride]"""
-							.formatted(testClass.getName(), AbstractTestBeanIntegrationTestCase.Pojo.class.getName()))));
+					message("Found 2 competing static methods named anotherBean() or thirdBean() in %s with return type %s"
+							.formatted(Method1InSupertypeAndMethod2InTypeTestCase.class.getName(), Pojo.class.getName()))));
 	}
 
 
@@ -69,11 +65,11 @@ void test() {
 
 	static class Method1InSupertypeAndMethod2InTypeTestCase extends AbstractTestBeanIntegrationTestCase {
 
-		static Pojo someBeanTestOverride() {
+		static Pojo someBean() {
 			return new FakePojo("ignored");
 		}
 
-		static Pojo anotherBeanTestOverride() {
+		static Pojo anotherBean() {
 			return new FakePojo("sub2");
 		}
 

spring-test/src/test/java/org/springframework/test/context/bean/override/convention/FailingTestBeanIntegrationTests.java

@@ -66,25 +66,21 @@ void testBeanFailingNoExplicitNameBean() {
 
 	@Test
 	void testBeanFailingNoImplicitMethod() {
-		Class<?> testClass = ExplicitTestOverrideMethodNotPresentTestCase.class;
+		Class<?> testClass = ExplicitOverrideMethodNotPresentTestCase.class;
 		EngineTestKitUtils.executeTestsForClass(testClass).assertThatEvents().haveExactly(1,
 			finishedWithFailure(
 					instanceOf(IllegalStateException.class),
-					message("""
-						Failed to find a static test bean factory method in %s with return type \
-						java.lang.String whose name matches one of the supported candidates \
-						[notPresent]""".formatted(testClass.getName()))));
+					message("No static method found named notPresent() in %s with return type %s"
+							.formatted(testClass.getName(), String.class.getName()))));
 	}
 
 	@Test
 	void testBeanFailingNoExplicitMethod() {
-		Class<?> testClass = ImplicitTestOverrideMethodNotPresentTestCase.class;
+		Class<?> testClass = ImplicitOverrideMethodNotPresentTestCase.class;
 		EngineTestKitUtils.executeTestsForClass(testClass).assertThatEvents().haveExactly(1,
 			finishedWithFailure(instanceOf(IllegalStateException.class),
-					message("""
-						Failed to find a static test bean factory method in %s with return type \
-						java.lang.String whose name matches one of the supported candidates \
-						[fieldTestOverride]""".formatted(testClass.getName()))));
+					message("No static method found named field() in %s with return type %s"
+							.formatted(testClass.getName(), String.class.getName()))));
 	}
 
 	@Test
@@ -111,7 +107,7 @@ void test() {
 			fail("should fail earlier");
 		}
 
-		static String noOriginalBeanTestOverride() {
+		static String noOriginalBean() {
 			return "should be ignored";
 		}
 	}
@@ -127,13 +123,13 @@ void test() {
 			fail("should fail earlier");
 		}
 
-		static String notPresentTestOverride() {
+		static String notPresent() {
 			return "should be ignored";
 		}
 	}
 
 	@SpringJUnitConfig
-	static class ExplicitTestOverrideMethodNotPresentTestCase {
+	static class ExplicitOverrideMethodNotPresentTestCase {
 
 		@TestBean(methodName = "notPresent")
 		String field;
@@ -145,9 +141,9 @@ void test() {
 	}
 
 	@SpringJUnitConfig
-	static class ImplicitTestOverrideMethodNotPresentTestCase {
+	static class ImplicitOverrideMethodNotPresentTestCase {
 
-		@TestBean // expects fieldTestOverride method
+		@TestBean // expects field method
 		String field;
 
 		@Test
@@ -167,7 +163,7 @@ void test() {
 			fail("should fail earlier");
 		}
 
-		static String fieldTestOverride() {
+		static String field() {
 			return "should be ignored";
 		}
 

spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanByTypeIntegrationTests.java

@@ -51,7 +51,7 @@ public class TestBeanByTypeIntegrationTests {
 	@CustomQualifier
 	StringBuilder anyNameForStringBuilder2;
 
-	static ExampleService anyNameForServiceTestOverride() {
+	static ExampleService anyNameForService() {
 		return new RealExampleService("Mocked greeting");
 	}