Revise "Introduce class-level execution phases for @⁠Sql"

This commit revises the previous commit as follows.

- Remove hasTestMethod() from TestContext and instead introduce a new
  variant of createDelegatingTransactionAttribute() in
  TestContextTransactionUtils which accepts a boolean
  `includeMethodName` flag.

- Add missing @⁠TestMethodOrder declaration to
  AfterTestClassSqlScriptsTests to ensure that the test methods are
  always executed in the required order.

- Polish Javadoc and add missing @⁠since tags.

Closes gh-27285

spring-test/src/main/java/org/springframework/test/context/TestContext.java

@@ -42,7 +42,6 @@ import org.springframework.test.annotation.DirtiesContext.HierarchyMode;
  * override {@link #setMethodInvoker(MethodInvoker)} and {@link #getMethodInvoker()}.
  *
  * @author Sam Brannen
- * @author Andreas Ahlenstorf
  * @since 2.5
  * @see TestContextManager
  * @see TestExecutionListener
@@ -111,25 +110,6 @@ public interface TestContext extends AttributeAccessor, Serializable {
 	 */
 	Object getTestInstance();
 
-	/**
-	 * Tests whether a test method is part of this test context. Returns
-	 * {@code true} if this context has a current test method, {@code false}
-	 * otherwise.
-	 *
-	 * <p>The default implementation of this method always returns {@code false}.
-	 * Custom {@code TestContext} implementations are therefore highly encouraged
-	 * to override this method with a more meaningful implementation. Note that
-	 * the standard {@code TestContext} implementation in Spring overrides this
-	 * method appropriately.
-	 * @return {@code true} if the test execution has already entered a test
-	 * method
-	 * @since 6.1
-	 * @see #getTestMethod()
-	 */
-	default boolean hasTestMethod() {
-		return false;
-	}
-
 	/**
 	 * Get the current {@linkplain Method test method} for this test context.
 	 * <p>Note: this is a mutable property.

spring-test/src/main/java/org/springframework/test/context/jdbc/Sql.java

@@ -33,11 +33,12 @@ import org.springframework.core.annotation.AliasFor;
  *
  * <p>Method-level declarations override class-level declarations by default,
  * but this behavior can be configured via {@link SqlMergeMode @SqlMergeMode}.
- * However, this does not apply to class-level declarations that use
- * {@link ExecutionPhase#BEFORE_TEST_CLASS} or
- * {@link ExecutionPhase#AFTER_TEST_CLASS}. Such declarations are retained and
- * scripts and statements are executed once per class in addition to any
- * method-level annotations.
+ * However, this does not apply to class-level declarations configured for the
+ * {@link ExecutionPhase#BEFORE_TEST_CLASS BEFORE_TEST_CLASS} or
+ * {@link ExecutionPhase#AFTER_TEST_CLASS AFTER_TEST_CLASS} execution phase. Such
+ * declarations cannot be overridden, and the corresponding scripts and statements
+ * will be executed once per class in addition to any method-level scripts and
+ * statements.
  *
  * <p>Script execution is performed by the {@link SqlScriptsTestExecutionListener},
  * which is enabled by default.
@@ -169,13 +170,15 @@ public @interface Sql {
 
 		/**
 		 * The configured SQL scripts and statements will be executed
-		 * once <em>before</em> any test method is run.
+		 * once per test class <em>before</em> any test method is run.
+		 * @since 6.1
 		 */
 		BEFORE_TEST_CLASS,
 
 		/**
 		 * The configured SQL scripts and statements will be executed
-		 * once <em>after</em> any test method is run.
+		 * once per test class <em>after</em> all test methods have run.
+		 * @since 6.1
 		 */
 		AFTER_TEST_CLASS,
 

spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java

@@ -68,8 +68,8 @@ import static org.springframework.util.ResourceUtils.CLASSPATH_URL_PREFIX;
  * configured via the {@link Sql @Sql} annotation.
  *
  * <p>Class-level annotations that are constrained to a class-level execution
- * phase ({@link ExecutionPhase#BEFORE_TEST_CLASS} or
- * {@link ExecutionPhase#AFTER_TEST_CLASS}) will be run
+ * phase ({@link ExecutionPhase#BEFORE_TEST_CLASS BEFORE_TEST_CLASS} or
+ * {@link ExecutionPhase#AFTER_TEST_CLASS AFTER_TEST_CLASS}) will be run
  * {@linkplain #beforeTestClass(TestContext) once before all test methods} or
  * {@linkplain #afterTestMethod(TestContext) once after all test methods},
  * respectively. All other scripts and inlined statements will be executed
@@ -138,20 +138,22 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen
 	 * Execute SQL scripts configured via {@link Sql @Sql} for the supplied
 	 * {@link TestContext} once per test class <em>before</em> any test method
 	 * is run.
+	 * @since 6.1
 	 */
 	@Override
 	public void beforeTestClass(TestContext testContext) throws Exception {
-		executeBeforeOrAfterClassSqlScripts(testContext, ExecutionPhase.BEFORE_TEST_CLASS);
+		executeClassLevelSqlScripts(testContext, ExecutionPhase.BEFORE_TEST_CLASS);
 	}
 
 	/**
 	 * Execute SQL scripts configured via {@link Sql @Sql} for the supplied
 	 * {@link TestContext} once per test class <em>after</em> all test methods
 	 * have been run.
+	 * @since 6.1
 	 */
 	@Override
 	public void afterTestClass(TestContext testContext) throws Exception {
-		executeBeforeOrAfterClassSqlScripts(testContext, ExecutionPhase.AFTER_TEST_CLASS);
+		executeClassLevelSqlScripts(testContext, ExecutionPhase.AFTER_TEST_CLASS);
 	}
 
 	/**
@@ -189,11 +191,12 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen
 
 	/**
 	 * Execute class-level SQL scripts configured via {@link Sql @Sql} for the
-	 * supplied {@link TestContext} and the execution phases
-	 * {@link ExecutionPhase#BEFORE_TEST_CLASS} and
-	 * {@link ExecutionPhase#AFTER_TEST_CLASS}.
+	 * supplied {@link TestContext} and the supplied
+	 * {@link ExecutionPhase#BEFORE_TEST_CLASS BEFORE_TEST_CLASS} or
+	 * {@link ExecutionPhase#AFTER_TEST_CLASS AFTER_TEST_CLASS} execution phase.
+	 * @since 6.1
 	 */
-	private void executeBeforeOrAfterClassSqlScripts(TestContext testContext, ExecutionPhase executionPhase) {
+	private void executeClassLevelSqlScripts(TestContext testContext, ExecutionPhase executionPhase) {
 		Class<?> testClass = testContext.getTestClass();
 		executeSqlScripts(getSqlAnnotationsFor(testClass), testContext, executionPhase, true);
 	}
@@ -286,7 +289,7 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen
 			Sql sql, ExecutionPhase executionPhase, TestContext testContext, boolean classLevel) {
 
 		Assert.isTrue(classLevel || isValidMethodLevelPhase(sql.executionPhase()),
-				() -> "%s cannot be used on methods".formatted(sql.executionPhase()));
+				() -> "@SQL execution phase %s cannot be used on methods".formatted(sql.executionPhase()));
 
 		if (executionPhase != sql.executionPhase()) {
 			return;
@@ -302,10 +305,8 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen
 					.formatted(executionPhase, testContext.getTestClass().getName()));
 		}
 
-		Method testMethod = null;
-		if (testContext.hasTestMethod()) {
-			testMethod = testContext.getTestMethod();
-		}
+		boolean methodLevel = !classLevel;
+		Method testMethod = (methodLevel ? testContext.getTestMethod() : null);
 
 		String[] scripts = getScripts(sql, testContext.getTestClass(), testMethod, classLevel);
 		List<Resource> scriptResources = TestContextResourceUtils.convertToResourceList(
@@ -357,7 +358,7 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen
 			int propagation = (newTxRequired ? TransactionDefinition.PROPAGATION_REQUIRES_NEW :
 					TransactionDefinition.PROPAGATION_REQUIRED);
 			TransactionAttribute txAttr = TestContextTransactionUtils.createDelegatingTransactionAttribute(
-					testContext, new DefaultTransactionAttribute(propagation));
+					testContext, new DefaultTransactionAttribute(propagation), methodLevel);
 			new TransactionTemplate(txMgr, txAttr).executeWithoutResult(s -> populator.execute(finalDataSource));
 		}
 	}
@@ -458,7 +459,8 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen
 
 	private static boolean isValidMethodLevelPhase(ExecutionPhase executionPhase) {
 		// Class-level phases cannot be used on methods.
-		return executionPhase == ExecutionPhase.BEFORE_TEST_METHOD ||
-				executionPhase == ExecutionPhase.AFTER_TEST_METHOD;
+		return (executionPhase == ExecutionPhase.BEFORE_TEST_METHOD ||
+				executionPhase == ExecutionPhase.AFTER_TEST_METHOD);
 	}
+
 }

spring-test/src/main/java/org/springframework/test/context/support/DefaultTestContext.java

@@ -41,7 +41,6 @@ import org.springframework.util.StringUtils;
  * @author Sam Brannen
  * @author Juergen Hoeller
  * @author Rob Harrop
- * @author Andreas Ahlenstorf
  * @since 4.0
  */
 @SuppressWarnings("serial")
@@ -167,11 +166,6 @@ public class DefaultTestContext implements TestContext {
 		return testInstance;
 	}
 
-	@Override
-	public boolean hasTestMethod() {
-		return this.testMethod != null;
-	}
-
 	@Override
 	public final Method getTestMethod() {
 		Method testMethod = this.testMethod;

spring-test/src/main/java/org/springframework/test/context/transaction/TestContextTransactionUtils.java

@@ -228,8 +228,7 @@ public abstract class TestContextTransactionUtils {
 	/**
 	 * Create a delegating {@link TransactionAttribute} for the supplied target
 	 * {@link TransactionAttribute} and {@link TestContext}, using the names of
-	 * the test class and test method (if available) to build the name of the
-	 * transaction.
+	 * the test class and test method to build the name of the transaction.
 	 * @param testContext the {@code TestContext} upon which to base the name
 	 * @param targetAttribute the {@code TransactionAttribute} to delegate to
 	 * @return the delegating {@code TransactionAttribute}
@@ -237,9 +236,27 @@ public abstract class TestContextTransactionUtils {
 	public static TransactionAttribute createDelegatingTransactionAttribute(
 			TestContext testContext, TransactionAttribute targetAttribute) {
 
+		return createDelegatingTransactionAttribute(testContext, targetAttribute, true);
+	}
+
+	/**
+	 * Create a delegating {@link TransactionAttribute} for the supplied target
+	 * {@link TransactionAttribute} and {@link TestContext}, using the names of
+	 * the test class and test method (if requested) to build the name of the
+	 * transaction.
+	 * @param testContext the {@code TestContext} upon which to base the name
+	 * @param targetAttribute the {@code TransactionAttribute} to delegate to
+	 * @param includeMethodName {@code true} if the test method's name should be
+	 * included in the name of the transaction
+	 * @return the delegating {@code TransactionAttribute}
+	 * @since 6.1
+	 */
+	public static TransactionAttribute createDelegatingTransactionAttribute(
+			TestContext testContext, TransactionAttribute targetAttribute, boolean includeMethodName) {
+
 		Assert.notNull(testContext, "TestContext must not be null");
 		Assert.notNull(targetAttribute, "Target TransactionAttribute must not be null");
-		return new TestContextTransactionAttribute(targetAttribute, testContext);
+		return new TestContextTransactionAttribute(targetAttribute, testContext, includeMethodName);
 	}
 
 
@@ -248,10 +265,12 @@ public abstract class TestContextTransactionUtils {
 
 		private final String name;
 
-		public TestContextTransactionAttribute(TransactionAttribute targetAttribute, TestContext testContext) {
+		public TestContextTransactionAttribute(
+				TransactionAttribute targetAttribute, TestContext testContext, boolean includeMethodName) {
+
 			super(targetAttribute);
 
-			if (testContext.hasTestMethod()) {
+			if (includeMethodName) {
 				this.name = ClassUtils.getQualifiedMethodName(testContext.getTestMethod(), testContext.getTestClass());
 			}
 			else {

spring-test/src/test/java/org/springframework/test/context/jdbc/AfterTestClassSqlScriptsTests.java

@@ -18,55 +18,66 @@ package org.springframework.test.context.jdbc;
 
 import javax.sql.DataSource;
 
+import org.junit.jupiter.api.MethodOrderer.OrderAnnotation;
 import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
 
-import org.springframework.core.Ordered;
 import org.springframework.jdbc.BadSqlGrammarException;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.test.annotation.Commit;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.TestContext;
-import org.springframework.test.context.TestExecutionListener;
 import org.springframework.test.context.TestExecutionListeners;
+import org.springframework.test.context.jdbc.AfterTestClassSqlScriptsTests.VerifySchemaDroppedListener;
+import org.springframework.test.context.jdbc.Sql.ExecutionPhase;
 import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
+import org.springframework.test.context.support.AbstractTestExecutionListener;
 import org.springframework.test.context.transaction.TestContextTransactionUtils;
 
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.springframework.test.context.TestExecutionListeners.MergeMode.MERGE_WITH_DEFAULTS;
+import static org.springframework.test.context.jdbc.Sql.ExecutionPhase.AFTER_TEST_CLASS;
 
 /**
- * Verifies that {@link Sql @Sql} with {@link Sql.ExecutionPhase#AFTER_TEST_CLASS} is run after all tests in the class
- * have been run.
+ * Verifies that {@link Sql @Sql} with {@link ExecutionPhase#AFTER_TEST_CLASS}
+ * is run after all tests in the class have been run.
  *
  * @author Andreas Ahlenstorf
+ * @author Sam Brannen
  * @since 6.1
  */
 @SpringJUnitConfig(PopulatedSchemaDatabaseConfig.class)
-@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS)
-@Sql(value = {"drop-schema.sql"}, executionPhase = Sql.ExecutionPhase.AFTER_TEST_CLASS)
-@TestExecutionListeners(
-		value = AfterTestClassSqlScriptsTests.VerifyTestExecutionListener.class,
-		mergeMode = TestExecutionListeners.MergeMode.MERGE_WITH_DEFAULTS
-)
+@TestMethodOrder(OrderAnnotation.class)
+@DirtiesContext
+@Sql(scripts = "drop-schema.sql", executionPhase = AFTER_TEST_CLASS)
+@Commit
+@TestExecutionListeners(listeners = VerifySchemaDroppedListener.class, mergeMode = MERGE_WITH_DEFAULTS)
 class AfterTestClassSqlScriptsTests extends AbstractTransactionalTests {
 
 	@Test
 	@Order(1)
-	@Sql(scripts = "data-add-catbert.sql")
-	@Commit
+	@Sql("data-add-catbert.sql")
 	void databaseHasBeenInitialized() {
 		assertUsers("Catbert");
 	}
 
 	@Test
 	@Order(2)
-	@Sql(scripts = "data-add-dogbert.sql")
-	@Commit
+	@Sql("data-add-dogbert.sql")
 	void databaseIsNotWipedBetweenTests() {
 		assertUsers("Catbert", "Dogbert");
 	}
 
-	static class VerifyTestExecutionListener implements TestExecutionListener, Ordered {
+
+	static class VerifySchemaDroppedListener extends AbstractTestExecutionListener {
+
+		@Override
+		public int getOrder() {
+			// Must run before DirtiesContextTestExecutionListener. Otherwise, the
+			// old data source will be removed and replaced with a new one.
+			return 3001;
+		}
 
 		@Override
 		public void afterTestClass(TestContext testContext) throws Exception {
@@ -74,14 +85,9 @@ class AfterTestClassSqlScriptsTests extends AbstractTransactionalTests {
 			JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource);
 
 			assertThatExceptionOfType(BadSqlGrammarException.class)
-					.isThrownBy(() -> jdbcTemplate.queryForList("SELECT name FROM user", String.class));
-		}
-
-		@Override
-		public int getOrder() {
-			// Must run before DirtiesContextTestExecutionListener. Otherwise, the old data source will be removed and
-			// replaced with a new one.
-			return 3001;
+					.isThrownBy(() -> jdbcTemplate.queryForList("SELECT name FROM user", String.class))
+					.withMessageContaining("user");
 		}
 	}
+
 }

spring-test/src/test/java/org/springframework/test/context/jdbc/BeforeTestClassSqlScriptsTests.java

@@ -19,21 +19,24 @@ package org.springframework.test.context.jdbc;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.test.annotation.DirtiesContext;
+import org.springframework.test.context.jdbc.Sql.ExecutionPhase;
 import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
 
+import static org.springframework.test.context.jdbc.Sql.ExecutionPhase.BEFORE_TEST_CLASS;
 import static org.springframework.test.context.jdbc.SqlMergeMode.MergeMode.MERGE;
 import static org.springframework.test.context.jdbc.SqlMergeMode.MergeMode.OVERRIDE;
 
 /**
- * Verifies that {@link Sql @Sql} with {@link Sql.ExecutionPhase#BEFORE_TEST_CLASS} is run before all tests in the class
- * have been run.
+ * Verifies that {@link Sql @Sql} with {@link ExecutionPhase#BEFORE_TEST_CLASS}
+ * is run before all tests in the class have been run.
  *
  * @author Andreas Ahlenstorf
+ * @author Sam Brannen
  * @since 6.1
  */
-@SpringJUnitConfig(classes = EmptyDatabaseConfig.class)
+@SpringJUnitConfig(EmptyDatabaseConfig.class)
 @DirtiesContext
-@Sql(value = {"schema.sql", "data-add-catbert.sql"}, executionPhase = Sql.ExecutionPhase.BEFORE_TEST_CLASS)
+@Sql(scripts = {"schema.sql", "data-add-catbert.sql"}, executionPhase = BEFORE_TEST_CLASS)
 class BeforeTestClassSqlScriptsTests extends AbstractTransactionalTests {
 
 	@Test
@@ -52,7 +55,7 @@ class BeforeTestClassSqlScriptsTests extends AbstractTransactionalTests {
 	@Sql({"data-add-dogbert.sql"})
 	@SqlMergeMode(OVERRIDE)
 	void overrideDoesNotAffectClassLevelPhase() {
-		assertUsers("Dogbert", "Catbert");
+		assertUsers("Catbert", "Dogbert");
 	}
 
 }

spring-test/src/test/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListenerTests.java

@@ -30,6 +30,8 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
+import static org.springframework.test.context.jdbc.Sql.ExecutionPhase.AFTER_TEST_CLASS;
+import static org.springframework.test.context.jdbc.Sql.ExecutionPhase.BEFORE_TEST_CLASS;
 
 /**
  * Unit tests for {@link SqlScriptsTestExecutionListener}.
@@ -58,7 +60,6 @@ class SqlScriptsTestExecutionListenerTests {
 	void missingValueAndScriptsAndStatementsAtMethodLevel() throws Exception {
 		Class<?> clazz = MissingValueAndScriptsAndStatementsAtMethodLevel.class;
 		BDDMockito.<Class<?>> given(testContext.getTestClass()).willReturn(clazz);
-		given(testContext.hasTestMethod()).willReturn(true);
 		given(testContext.getTestMethod()).willReturn(clazz.getDeclaredMethod("foo"));
 
 		assertExceptionContains(clazz.getSimpleName() + ".foo" + ".sql");
@@ -109,24 +110,22 @@ class SqlScriptsTestExecutionListenerTests {
 	void beforeTestClassOnMethod() throws Exception {
 		Class<?> clazz = ClassLevelExecutionPhaseOnMethod.class;
 		BDDMockito.<Class<?>> given(testContext.getTestClass()).willReturn(clazz);
-		given(testContext.hasTestMethod()).willReturn(true);
 		given(testContext.getTestMethod()).willReturn(clazz.getDeclaredMethod("beforeTestClass"));
 
 		assertThatIllegalArgumentException()
 				.isThrownBy(() -> listener.beforeTestMethod(testContext))
-				.withMessage("BEFORE_TEST_CLASS cannot be used on methods");
+				.withMessage("@SQL execution phase BEFORE_TEST_CLASS cannot be used on methods");
 	}
 
 	@Test
 	void afterTestClassOnMethod() throws Exception {
 		Class<?> clazz = ClassLevelExecutionPhaseOnMethod.class;
 		BDDMockito.<Class<?>> given(testContext.getTestClass()).willReturn(clazz);
-		given(testContext.hasTestMethod()).willReturn(true);
 		given(testContext.getTestMethod()).willReturn(clazz.getDeclaredMethod("afterTestClass"));
 
 		assertThatIllegalArgumentException()
 				.isThrownBy(() -> listener.beforeTestMethod(testContext))
-				.withMessage("AFTER_TEST_CLASS cannot be used on methods");
+				.withMessage("@SQL execution phase AFTER_TEST_CLASS cannot be used on methods");
 	}
 
 	private void assertExceptionContains(String msg) throws Exception {
@@ -175,12 +174,13 @@ class SqlScriptsTestExecutionListenerTests {
 
 	static class ClassLevelExecutionPhaseOnMethod {
 
-		@Sql(scripts = "foo.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_CLASS)
+		@Sql(scripts = "foo.sql", executionPhase = BEFORE_TEST_CLASS)
 		public void beforeTestClass() {
 		}
 
-		@Sql(scripts = "foo.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_CLASS)
+		@Sql(scripts = "foo.sql", executionPhase = AFTER_TEST_CLASS)
 		public void afterTestClass() {
 		}
 	}
+
 }