Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TransactionalTestExecutionListener regression in 4.1 [SPR-12554] #17156

Closed
spring-projects-issues opened this issue Dec 18, 2014 · 6 comments
Closed

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Dec 18, 2014

Alan Boshier opened SPR-12554 and commented

This is similar but not identical to #16859.

Our base class tests are annotated as follows:

@TestExecutionListeners({ 
    DependencyInjectionTestExecutionListener.class,
    DirtiesContextTestExecutionListener.class,
    TransactionalTestExecutionListener.class,
    DbUnitTestExecutionListener.class 
    })
@DbUnitConfiguration(
      databaseOperationLookup=ExtendedDatabaseOperationLookup.class,
                dataSetLoader=ColumnSensingDataSetLoader.class)
public abstract class FooTestBase
                    extends AbstractTransactionalJUnit4SpringContextTests
                    implements FooTestInterface

... with an annotated subclass:

@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(value = "/foo-context-test.xml")
@TransactionConfiguration(defaultRollback = true)
@Transactional
public class FooTest extends FooTestBase
{

In moving from Spring 4.0 to 4.1 we are seeing many of our tests fail with:

java.lang.IllegalStateException: Cannot start a new transaction without ending the existing transaction.
        at org.springframework.test.context.transaction.TransactionalTestExecutionListener.beforeTestMethodTransactionalTestExecutionListener.java:175)
        at org.springframework.test.context.TestContextManager.beforeTestMethod(TestContextManager.java:249)
        at org.springframework.test.context.junit4.statements.RunBeforeTestMethodCallbacks.evaluate(RunBeforeTestMethodCallbacks.java:72)
        at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
        at org.springframework.test.context.junit4.statements.RunAfterTestMethodCallbacks.evaluate(RunAfterTestMethodCallbacks.java:82)
        at org.springframework.test.context.junit4.statements.SpringRepeat.evaluate(SpringRepeat.java:73)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
        at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:217)
        at org.springframework.test.context.junit4.SpringJUnit4ClassRunner.runChild(SpringJUnit4ClassRunner.java:83)

Affects: 4.1.3

Issue Links:

  • #16859 Transactional test with TransactionalTestExecutionListener inheritance breaks backward compatibility ("duplicates")
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 18, 2014

Sam Brannen commented

Like with #16859, this is simply a configuration error. If this configuration previously worked for you, that was completely unintentional and actually unsupported. In other words, it was a bug that it ever worked since you effectively declare the TransactionalTestExecutionListener twice.

The correct configuration for your use case is the following:

@TestExecutionListeners(DbUnitTestExecutionListener.class)
@DbUnitConfiguration(
      databaseOperationLookup=ExtendedDatabaseOperationLookup.class,
                dataSetLoader=ColumnSensingDataSetLoader.class)
public abstract class FooTestBase
                    extends AbstractTransactionalJUnit4SpringContextTests
                    implements FooTestInterface { /* ... */ }

Regards,

Sam

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 18, 2014

Sam Brannen commented

Resolving this issue as Works as Designed.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 18, 2014

Alan Boshier commented

Hi Sam

Thanks for your quick response; just to check my understanding are you saying that if we have:

@TransactionConfiguration(defaultRollback = true)
@Transactional

on our subclass, then this is implicitly declaring a TransactionalTestExecutionListener?

In your suggested revisions to our base class, along with removing TransactionTestExecutionListerner, you've also removed DependencyInjectionTestExecutionListener and DirtiesContextTestExecutionListener. Was that intentional?

Thanks

Alan

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 18, 2014

Sam Brannen commented

then this is implicitly declaring a TransactionalTestExecutionListener?

No. TransactionalTestExecutionListener is registered because AbstractTransactionalJUnit4SpringContextTests explicitly declares it.

In your suggested revisions to our base class, along with removing TransactionTestExecutionListerner, you've also removed DependencyInjectionTestExecutionListener and DirtiesContextTestExecutionListener. Was that intentional?

Yes, this was intentional, because AbstractJUnit4SpringContextTests explicitly declares those listeners and AbstractTransactionalJUnit4SpringContextTests extends AbstractJUnit4SpringContextTests.

I assumed you were aware of the inheritance of listeners, but that assumption was apparently incorrect. So I apologize for that.

For further details, please consult the Javadoc for AbstractJUnit4SpringContextTests and AbstractTransactionalJUnit4SpringContextTests as well as the TestExecutionListener configuration section of the reference manual.

Note as well that your declaration of @Transactional and @TransactionConfiguration(defaultRollback = true) is completely unnecessary:

  1. @Transactional is already declared on AbstractTransactionalJUnit4SpringContextTests and therefore inherited.
  2. The defaultRollback flag in @TransactionConfiguration is true by default. Thus there is no reason to override the default with the default.

In summary, the following is the simplest, cleanest configuration for your use case:

@TestExecutionListeners(DbUnitTestExecutionListener.class)
@DbUnitConfiguration(
      databaseOperationLookup=ExtendedDatabaseOperationLookup.class,
      dataSetLoader=ColumnSensingDataSetLoader.class)
public abstract class FooTestBase
                    extends AbstractTransactionalJUnit4SpringContextTests
                    implements FooTestInterface { /* ... */ }

... with an annotated subclass:

@ContextConfiguration("/foo-context-test.xml")
public class FooTest extends FooTestBase { /* ... */ }

Due to the inheritance of TestExecutionListener configuration (see the inheritListeners attribute in @TestExecutionListeners), the listeners configured for FooTest will be (as of Spring Framework 4.1):

  1. ServletTestExecutionListener
  2. DependencyInjectionTestExecutionListener
  3. DirtiesContextTestExecutionListener
  4. TransactionalTestExecutionListener
  5. SqlScriptsTestExecutionListener
  6. DbUnitTestExecutionListener

You can verify this by setting the log level for the org.springframework.test.context.support logger category to INFO.

FYI: redeclaring @RunWith(SpringJUnit4ClassRunner.class) is also unnecessary, since AbstractJUnit4SpringContextTests is already annotated with that.

Regards,

Sam

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 18, 2014

Alan Boshier commented

Sam - many thanks for taking the trouble to explain everything so clearly; much appreciated!

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Dec 18, 2014

Sam Brannen commented

You're welcome!

And... enjoy the reduced number of lines of configuration. ;)

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants