Reification of associations is not compatible with transactional tests
timwaters opened this issue · 12 comments
Hello, I'm using Test Unit / Minitest on a new Rails 4.2 application on Ruby 2.2 with Paper Trail 4.0.0.rc1. I am seeing associations being restored in the application, in the console in both postgres and sqlite dbs. However within a test the associations are not being restored properly. I wouldn't be surprised if I wrote the test wrong!! but given a search of the Issues there may be something going on.
The simple application is here: https://github.com/timwaters/paper_trail_testing and here are some of the relevant parts:
class Comment < ActiveRecord::Base
belongs_to :article
has_paper_trail
after_save :touch_article
def touch_article
article.touch_with_version
end
end
class Article < ActiveRecord::Base
has_many :comments
has_paper_trail
end
and test
require 'test_helper'
class ArticleTest < ActiveSupport::TestCase
a = Article.new
a.title = "first"
a.text = "text stuff"
a.save
c = Comment.new
c.text = "this is a comment"
c.article = a
c.save
c = Comment.last
c.text = "changed"
c.save
article = Article.last
refied_article = article.reload.versions.last.reify(:has_many => true)
puts refied_article.comments.inspect #<ActiveRecord::Associations::CollectionProxy [#<Comment id: 980191006, text: "this is a comment", ....
puts refied_article.comments.first.text == "this is a comment" #TRUE
test "adding versioning " do
with_versioning do
a = Article.new
a.title = "first"
a.text = "text stuff"
a.save
c = Comment.new
c.text = "this is a comment"
c.article = a
c.save
c = Comment.last
c.text = "changed"
c.save
article = Article.last
refied_article = article.reload.versions.last.reify(:has_many => true)
puts refied_article.comments.inspect ##<ActiveRecord::Associations::CollectionProxy []>
assert refied_article.comments.first.text == "this is a comment" #undefined method `text' for nil:NilClass
end
end
end
The first block gets executed outside of the test, and performs as expected. If I put the same code in the block the comments do not get restored. Indeed, there are no comments. (Reloading the comments gives the latest comment, as expected)
The article is reified to the correct version in each.
The same behaviour works if I out the test code outside of the with_versioning block, if Paper Trail is enabled globally, or disabled globally and just enabled in the specific test, and wrapping the changes in Article.transaction and changing the article at the same time also does not seem to make much difference. It also seems to make no difference adding PaperTrail::Rails::Engine.eager_load! to an initializer.
The test log can be found here: https://gist.github.com/timwaters/e26b4e2ae0583b3fc5a8 and to my eyes it appears as if it's doing the right thing equally in both cases, just not loading up the correct association properly. (Disabling fixtures doesn't seem to change things either)
Many thanks for your time.
I'm getting the same empty associations in a controller test also. https://github.com/timwaters/paper_trail_testing/blob/master/test/controllers/versions_controller_test.rb
As with the unit test, in the actual app, associations are being restored.
Could this be related to #492 or is it about the test environment?
Thanks for the failing tests, Tim. I've pulled down timwaters/paper_trail_testing
and I'm taking a look.
@batter Here's what I've learned running Tim's excellent unit test in a debugger.
In Tim's first block, outside of the test
,
- each
create
runs in a separate transaction, - causing each record in
version_associations
to have a differenttransaction_id
. - When
reify_has_many_directly
runs itsselect min(version_id) .. where .. or transaction_id = ?
it only finds the record representing anupdate
.
In Tim's second block, inside the test
,
- each
create
runs in the same transaction (becauseuse_transactional_fixtures
is true by default), - causing each record in
version_associations
to have the sametransaction_id
. - When
reify_has_many_directly
runs itsselect min(version_id) ..
it finds thecreate
. options[:mark_for_destruction]
is false, so thecreate
is discarded
For fun, try setting self.use_transactional_fixtures = false
in Tim's unit test, and it'll pass.
@timwaters Without turning off use_transactional_fixtures
you could simulate separate transactions by calling PaperTrail.transaction_id=
. That should make your test pass.
@batter Have you had a chance to look at this? I'm starting to wonder if this should continue to hold up our 4.0 release.
Here's my attempt at a failing test that we could use in our test suite.
--- a/test/unit/model_test.rb
+++ b/test/unit/model_test.rb
@@ -1504,7 +1504,7 @@ class HasPaperTrailModelTransactionalTest < ActiveSupport::TestCase
context 'updated before the associated was created' do
setup do
@customer.update_attributes! :name => 'customer_1'
- @customer.orders.create! :order_date => Date.today
+ @order = @customer.orders.create! :order_date => "Thursday"
end
@@ -1517,6 +1517,13 @@ class HasPaperTrailModelTransactionalTest < ActiveSupport::TestCase
should 'not persist changes to the live association' do
assert_not_equal [], @customer.orders(true)
end
+
+ should 'demonstrate issue 542' do
+ @order.update_attributes! :order_date => "Friday"
+ assert_equal "Thursday", @order.versions.last.reify.order_date
+ reified_customer = @customer.versions.last.reify(:has_many => true)
+ assert_equal "Thursday", reified_customer.orders.first.try(:order_date)
+ end
end
It fails with:
Expected: "Thursday"
Actual: nil
Is this a useful failing test, or do I just not understand reified has_many associations?
I haven't had a chance to look at this yet, I will try to take a peek and run the tests locally by EOD today
Hmm... just read through this a little more closely, and dealing with complex nested associations and "playbacks" in transactional scenarios is indeed somewhat tricky.
So I just cloned down the sample project provided by @timwaters, and upon close inspection it appears that the gem is actually behaving as expected, and that the issue appears to be the fact that the after_save
callback attached to the Comment
class is being executed after a comment modification is persisted to the database, and thus the Article's new timestamp (and thus also the version generated by the call to touch_by_version
) is timestamped slightly after the time the comment's timestamp update has been registered.
If you wanted to make your setup work, I would recommend changing your Comment
class to look something like this:
class Comment < ActiveRecord::Base
belongs_to :article
has_paper_trail
after_save :touch_article
def touch_article
article.update_attribute(:updated_at, self.updated_at)
end
end
That would ensure the timestamp will match for both the latest Comment
and Article
records... that being said, I just gave that a whirl and that also doesn't appear to be reifying as I would expect when executed in the console in sandbox mode (where everything is run inside of a transaction), so this does indeed appear to be an issue surrounding transactions and associations.
Update:
- Reification of associations is not compatible with transactional tests, aka. transactional fixtures.
- This is an issue we'd like to fix, but are stumped.
- We welcome suggestions ❤️
One of possible workaround if you are using database_cleaner:
RSpec.configure do |config|
config.use_transactional_fixtures = false
config.before(:suite) do
DatabaseCleaner.clean_with(:truncation)
end
config.before(:each) do
DatabaseCleaner.strategy = :transaction
end
config.before(:each, versioning: true) do
DatabaseCleaner.strategy = :truncation
end
config.before(:each) do
DatabaseCleaner.start
end
config.append_after(:each) do
DatabaseCleaner.clean
end
end
Didn't test it, but may work.
Closing due to inactivity. This will remain listed in the readme with the other many known issues surrounding the experimental association tracking feature.
We have been able to work around our issues with this by using the Transactional Capybara gem - it may not be generally applicable, but it resolved our issues. We use it for testing published vs draft versions of models.