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

Problem with transaction & rollback behaviour in integration tests #929

Closed
3 of 4 tasks
sagrawal31 opened this issue May 2, 2017 · 5 comments
Closed
3 of 4 tasks

Comments

@sagrawal31
Copy link

Task List

  • Steps to reproduce provided
  • Stacktrace (if present) provided
  • Example that reproduces the problem uploaded to Github
  • Full description of the issue provided (see below)

Steps to Reproduce

  1. Clone the repo sagrawal31/integration-tests git clone [email protected]:sagrawal31/integration-tests.git
  2. Run all integration tests grails test-app -integration

Expected Behaviour

All test cases should pass.

Actual Behaviour

Data created in earlier test cases are not rolled back hence two test cases fails.

Environment Information

  • Operating System: MacOS Sierra
  • GORM Version: 6.1.2
  • Grails Version (if using Grails): 3.2.8 (Tried with 3.2.9 as well)
  • JDK Version: 1.8.0_91

Example Application

https://github.com/sagrawal31/integration-tests/commits

I'm aware of the recent GORM changes that everything needs to be in a Transaction. And also aware of using @Rollback annotations (from Grails & Spring) for writing test methods. Based on these understanding, I've written integration tests for controllers & services. Please see commit by commit in the above link. Here is what is happening:

  1. If I'm creating test data in test case for controllers and if I don't wrap it with withNewTransaction then those test data aren't available in the source code while running the test case of a controller
  2. In case of writing test case for a service, the above works just fine
  3. If I use withNewTransaction for creating test data in a controller's test case, then only it's available in the running source but then that test data is not automatically rolled back.

So basically, data is not rolled back if we don't create test data in withNewTrasaction for a controller spec but if create test data in withNewTransaction block then controller spec works but service spec starts failing due to previous data is not rolled back.

I've tried various solutions like using spring's @Rollback or GORM's @Transactional and @Rollback but nothing worked. I've tracked individual solutions in different commits but none of them made all the test cases happy.

I've already gone through previous related issues multiple times, for example, grails/grails-core#10504, grails/grails-core#9289, grails/grails-core#10556 etc but nothing worked for me for the past 3 days and I had to finally create this issue.

@graemerocher
Copy link
Member

graemerocher commented May 2, 2017

As far as I can see there is no way for you to do what you want for both the controller and service case.

The explanation is as follows. In the case where you use withNewTransaction, this works for the controller test case because it creates and commits a new transaction, but doesn't participate in the @Rollback transaction whatsoever. This makes the data visible to the controller, since the call to the controller via RestBuilder runs in a separate thread to the controller.

If you remove withNewTransaction this then makes the data setup part participate in the current @Rollback definition, but since this is isolated to the test thread, the data cannot be seen by the thread that executes when the request is sent by RestBuilder.

However, removing withNewTransaction makes the UserServiceSpec pass because now the transaction is participating in the definition of @Rollback.

The only way to do what you are doing and achieve rollback for both the controller test case and the service test case is to manually delete the data you created within the cleanupSpec method. There is no magic that GORM can do to make this happen automatically for you.

Therefore I am not exactly sure what solution or fix you anticipate us to make since as far as I can see nothing can be done to unify these two distinct approaches to testing.

You can of course refactor your IntegrationTestHelper trait to either create the data in a new transaction or not depending on some method argument, which may help, but again that relies on a user change, nothing that we can do.

@sagrawal31
Copy link
Author

Hmmm..so it's all about the thread. I got your point but a small confusion. When we create test data with or without withNewTransaction block, isn't that data is persisted to the db (whatever is configured for example MySQL)? If yes, then how a thread don't see data created from a current @Rollback annotation or a different thread?

Or the data created inside withNewTransaction is only persisted to the db but not that is created from a @Rollback definition?

@sagrawal31
Copy link
Author

Apart from the above, I updated my test suite to use transactional block just for the controller and removing it manually and all test works fine sagrawal31/integration-tests@6e35c4a. Is that what you wanted to say?

Also, if I add import grails.gorm.transactions.Transactional annotation to the UserServiceSpec, the data is not rolled back within the test methods of the same test class. Is that because @Rollback annotation creates its own transaction?

@graemerocher
Copy link
Member

graemerocher commented May 2, 2017

The data is persisted to the database yes, but it has only been written the transaction log. The @Rollback annotation manages the transaction so that at the database level the data is rolled back.

What this means is that the current transaction sees the data, but other threads that read data via different connections have no visibility on this data. This is quite simply how transactions work (read up on ACID).

If you use @Transactional instead of @Rollback then of course the data is committed and written the database and never rolled back so all of the test methods will see this data.

@sagrawal31
Copy link
Author

Great! Now clear with the setup. I read about ACID, good to know for future developement. Thanks @graemerocher

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

2 participants