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

Cannot rollback transactions in integration tests using spring's @Rollback annotation #10504

Closed
jonroler opened this issue Feb 28, 2017 · 5 comments

Comments

@jonroler
Copy link

Steps to Reproduce

  1. Create a service class annotation with the grails @transactional annotation that modifies the database.
  2. Create an integration test class annotated with the @Integration annotation and spring's (org.springframework.test.annotation.Rollback) @Rollback annotation that uses the service from step Grails fails to correctly identify Java Home in startGrails #1 to modify the database.
  3. Execute the test.
  4. Manually query the database. You will find that the changes made by the test in Untitled #3 were not rolled back as expected.

Expected Behaviour

The changes made in the integration test should be rolled back when the test completes, which is exactly what the grails docs say should happen: http://docs.grails.org/latest/guide/testing.html#integrationTesting (be sure to read the section "Using Spring's Rollback annotation"). Note that changing to the grails (grails.transaction.Rollback) annotation does cause the transaction to roll back as expected, but this is not an acceptable workaround because what I really want is for the changes in the test setup() method to be rolled back as well as the changes in the test method itself. The grails documentation says that using the spring @Rollback annotation is supposed to behave this way, but what I'm seeing is the spring @Rollback annotation does nothing. I have a very large grails project that I'm attempting to upgrade from grails 2.5.1 to grails 3.2.6. We have a ton of tests that rely on changes to the db made in the setup() methods rolling back at the end of the test method. It would be a big pain for us to go through all of our tests to rename the setup() method and add calls to this new method at the beginning of each test method.

Actual Behaviour

The changes made in the integration test are committed instead of rolled back. The behavior I'm seeing is completely different from the way the documentation says things should work, so I can only assume that this is a grails bug.

Environment Information

  • Operating System: Linux 4.9.7
  • Grails Version: 3.2.6
  • JDK Version: 1.8.0_121
  • Container Version (If Applicable): N/A

Example Application

Here is an application that has an integration test that reproduces this problem: https://github.com/jonroler/grails-rollback-issue. Please see the README.md file for instructions on how to run the test to reproduce the problem.

@graemerocher
Copy link
Member

The issue seems to be in Spring. The @Rollback annotation does not work without the presence of the @Transactional annotation. Adding @Transactional to AccessDBServiceSpec in addition to @Rollback resolves the issue to me.

If you have concerns about this feel free to report the issue to the Spring issue tracker since we have no control over the behaviour of Spring's annotations https://jira.spring.io/browse/SPR

@jonroler
Copy link
Author

jonroler commented Mar 1, 2017

Thanks for the quick response. Your comments make sense. However, if that's the way things are supposed to work, then I think the grails documentation should be changed to reflect your comments. Here's the example of using the spring Rollback annotation from the grails docs (see the "Using Spring’s Rollback annotation" section from http://docs.grails.org/latest/guide/testing.html#integrationTesting):

import grails.test.mixin.integration.Integration
import org.springframework.test.annotation.Rollback
import spock.lang.*

@Integration
@Rollback
class BookSpec extends Specification {

    void setup() {
        new Book(name: 'Grails in Action').save(flush: true)
    }

    void "test something"() {
        expect:
        Book.count() == 1
    }
}

You said that the @transactional annotation should be used when the spring @Rollback annotation is used. However, the example code from the grails documentation doesn't use the @transactional annotation at all.

Also, it wasn't clear from your comments whether the spring @transactional annotation (org.springframework.transaction.annotation.Transactional) or the grails @transactional annotation (grails.transaction.Transactional) should be used with the spring @Rollback annotation. From my testing, it appears that the spring @transactional annotation must be used with the spring @Rollback annotation in the test classes. If the grails @transactional annotation is used with the spring @Rollback annotation, then the transaction is still committed at the end of the test. That said, the rest of my code (i.e., non-testing code like service classes, etc.) that is being tested is using the grails @transactional annotation, so apparently it works to have the test class annotated with the spring @transactional annotation and the classes being tested (like service classes) annotated with the grails @transactional annotation.

I am having a hard time understanding why grails would provide a @Rollback annotation that doesn't rollback the effects of code in the setup() methods of the tests. The grails documentation addresses this by stating, "It isn’t possible to make grails.transaction.Rollback behave the same way as Spring’s Rollback annotation because grails.transaction.Rollback transforms the byte code of the class, eliminating the need for a proxy (which Spring’s version requires). This has the downside that you cannot implement it differently for different cases (as Spring does for testing)." I don't know anything about the implementation of the grails @Rollback annotation, and I certainly don't understand the "cannot implement it differently for different cases (as Spring does for testing)" part of the statement from the docs because I haven't seen the grails implementation of @Rollback. That said, would it be possible for grails to provide a @testRollBack annotation that automatically included setup() methods in the transaction of the test method that is using them? I'm not really sure where you'd normally want to use the @Rollback annotation for anything besides test code, so it sure seems like 99.99% of the time you'd want the db changes made in the setup() methods of tests to be rolled back along with the db changes made in each test method. In other words, from my point of view, there's really only one case (not a bunch of "different cases" like the grails docs imply) for the @Rollback annotation, and that is to rollback the db changes made in the setup() method when the transaction for each test method is rolled back. We are in the process of upgrading from grails 2.5.1 to grails 3.2.6, and this change to the behavior of the grails @Rollback annotation has broken > 95% of our integration tests (db changes made in setup() methods in grails 2.5.1 rolled back after each test method as expected). We have a lot of integration tests, so going through all of our tests and renaming the setup() method to setupData() (or something similar) and then calling setupData() at the beginning of each of our hundreds of test methods is kind of a pain to do. Besides that, using this setupData() method of doing things is a bit ugly. The normal way of writing a Spock integration test is to be able to use the setup() method to create data, but grails 3 has broken things so that you can't use spock in the standard way. That's my opinion, anyway,

In any case, it seems like the grails docs need to be updated to show a working example of using the spring @Rollback annotation. In addition it would be nice to see an improved explanation of why the grails @Rollback annotation can't be made to roll back changes made in the setup() methods of tests because the current explanation is not very clear to me. Since the behavior of the grails @Rollback annotation seems less than ideal, it would be nicer to have a better explanation.

@graemerocher
Copy link
Member

Agreed the docs could use updating. Note that I never said this is not an issue, just not an issue we can fix. It seems a change in behaviour in Spring because previously you could use @Rollback without the presence of @Transactional which no longer seems to be the case. Hence I suggest an issue report there.

As for the discussion about the behaviour of Grails' @Rollback within the context of tests. Something could probably be done to alter the compilation of tests that use @Rollback at the class level. Please report an issue here https://github.com/grails/grails-data-mapping/issues for this new feature request and we will see what we can do.

Thanks for the feedback.

@MichaelMorett
Copy link

MichaelMorett commented May 16, 2017

Hitting this exact problem now. The docs haven't been updated to include the "should be" solution. I could literally copy and paste jonroler's thoughts and concerns because as I was reading them, they described exactly what I was thinking.

@rmorrise
Copy link

I found this workaround, which worked for my use case:
https://objectpartners.com/2015/01/13/reset-your-h2-database-for-a-clean-state-between-functional-tests/

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

No branches or pull requests

4 participants