Skip to content

Commit

Permalink
Revise "Introduce class-level execution phases for @⁠Sql"
Browse files Browse the repository at this point in the history
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 spring-projectsgh-27285
  • Loading branch information
sbrannen committed Oct 4, 2023
1 parent 5aa2d05 commit 0afcb4d
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
* override {@link #setMethodInvoker(MethodInvoker)} and {@link #getMethodInvoker()}.
*
* @author Sam Brannen
* @author Andreas Ahlenstorf
* @since 2.5
* @see TestContextManager
* @see TestExecutionListener
Expand Down Expand Up @@ -111,25 +110,6 @@ default void publishEvent(Function<TestContext, ? extends ApplicationEvent> even
*/
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@
*
* <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.
Expand Down Expand Up @@ -169,13 +170,15 @@ enum ExecutionPhase {

/**
* 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,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@
* 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
Expand Down Expand Up @@ -138,20 +138,22 @@ public final int getOrder() {
* 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);
}

/**
Expand Down Expand Up @@ -189,11 +191,12 @@ public void processAheadOfTime(RuntimeHints runtimeHints, Class<?> testClass, Cl

/**
* 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);
}
Expand Down Expand Up @@ -286,7 +289,7 @@ private void executeSqlScripts(
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;
Expand All @@ -302,10 +305,8 @@ else if (logger.isDebugEnabled()) {
.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(
Expand Down Expand Up @@ -357,7 +358,7 @@ else if (logger.isDebugEnabled()) {
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));
}
}
Expand Down Expand Up @@ -458,7 +459,8 @@ private void registerClasspathResources(String[] paths, RuntimeHints runtimeHint

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);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
* @author Sam Brannen
* @author Juergen Hoeller
* @author Rob Harrop
* @author Andreas Ahlenstorf
* @since 4.0
*/
@SuppressWarnings("serial")
Expand Down Expand Up @@ -167,11 +166,6 @@ public final Object getTestInstance() {
return testInstance;
}

@Override
public boolean hasTestMethod() {
return this.testMethod != null;
}

@Override
public final Method getTestMethod() {
Method testMethod = this.testMethod;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,18 +228,35 @@ private static void logBeansException(TestContext testContext, BeansException ex
/**
* 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}
*/
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);
}


Expand All @@ -248,10 +265,12 @@ private static class TestContextTransactionAttribute extends DelegatingTransacti

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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,70 +18,76 @@

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 {
DataSource dataSource = TestContextTransactionUtils.retrieveDataSource(testContext, null);
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");
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,24 @@
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
Expand All @@ -52,7 +55,7 @@ void mergeDoesNotAffectClassLevelPhase() {
@Sql({"data-add-dogbert.sql"})
@SqlMergeMode(OVERRIDE)
void overrideDoesNotAffectClassLevelPhase() {
assertUsers("Dogbert", "Catbert");
assertUsers("Catbert", "Dogbert");
}

}
Loading

0 comments on commit 0afcb4d

Please sign in to comment.