strange issue with changing strategies and AR `after_commit` hooks

[agibralter]

Wow this was a hard thing to track down. It turns out that if ActiveRecord models with after_commit hooks get created in a transactional (DatabaseCleaner.strategy = :transaction) example and then a non-transactional example runs, all the left over objects' after_commit hooks will fire on the first "COMMIT" of the non-transactional example. This is because DatabaseCleaner does not call rollback_transaction_records...

I had to manually add this as an after hook in RSpec:

ActiveRecord::Base.connection.send(:rollback_transaction_records, true)

Perhaps something like this could go around here in the code?

https://github.com/bmabey/database_cleaner/blob/master/lib/database_cleaner/active_record/transaction.rb#L19

[agustinv]

I have this problem as well making switching strategies impossible and thus really hurting performance of tests. What is the ETA on this fix?

Thanks.

[bmabey]

Since DatabaseCleaner is calling a public API method, ActiveRecord::rollback_db_transaction, I'm kinda surprised this isn't already being taken care of by AR... maybe they have a better method we should be using.

Regardless, having read @agibralter's suggestion over again I think it makes sense to at least try his fix. I'm not familiar with after_commit hooks so I'm not sure if that is the best approach... It doesn't hurt to try it out.

@agustinv, could you try pointing your Gemfile to use the after_commit_fix branch to see if that fixes things for you?

[agibralter]

ah awesome, thank you @bmabey

[agustinv]

@bmabey Sorry for the delay. I just got to it today and the change did fix the issue for me. Thanks so much!

[bmabey]

Thanks, I've merged the fix into master.

[jfelchner]

Hey all, I'm having an issue with this. I'm currently running off master and this fix causes a problem if no DB rows were inserted during the spec (I think?).

Basically I'm getting:

undefined method `flatten' for nil:NilClass

From

# <snip>/activerecord-3.2.11/lib/active_record/connection_adapters/abstract/database_statements.rb:348:in `rollback_transaction_records'
# <snip>/database_cleaner-d6584cc394b3/lib/database_cleaner/active_record/transaction.rb:26:in `clean'

The relevant line from activerecord is here

My database_cleaner configuration is available here

And the spec I'm running is:

require 'my_spec_helper_that_loads_only_active_record_but_not_rails'
require 'my_class'

describe MyClass do
  it 'does stuff but no DB transactions' do
    MyClass.new.do_stuff 'cool'
  end
end

and MyClass contains only:

class MyClass
  def do_stuff(what_stuff)
    'wow'
  end
end

[l-schirrmeister]

I have the same issue.

In the active_record/connection_adapters/abstract/database_statements.rb the transaction method sets the needed instance variable

def transaction(options={})
  ...
  @_current_transaction_records ||= []
  ...
end

So it is only set if this method is really called, which seems not to be the case if no record is changed during the test.
Could be maybe a solution to change the DatabaseCleaner::ActiveRecord::Transaction#start method?

def start
  ...
  connection_class.connection.begin_db_transaction
  connection_class.connection.transaction
end

[agibralter]

Ah, yes, I forgot to mention that issue -- for non-AR specs, the nil exception gets raised. That's why in my after hook I have:

begin
  ActiveRecord::Base.connection.send(
    :rollback_transaction_records,
    true
  )
rescue
end

[jfelchner]

@bmabey do you agree that users should not be expected to know this hack?

[agibralter]

@jfeichner not sure I follow. Users should not have to worry about what's going on behind the scenes... I was just suggesting wrapping the rollback here in a begin/rescue...

[jfelchner]

@agibralter Ah, I was thinking you meant that when using database cleaner, that the user should need to add that snippet to make it work properly in all cases. My bad.

[chrishein]

I'm seeing the same issue here when no DB rows were inserted during the spec/scenario