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

Switching from :transaction to :truncation for some tests doesn't work #652

Open
wjessop opened this issue May 22, 2020 · 4 comments
Open

Comments

@wjessop
Copy link

wjessop commented May 22, 2020

So, I have some tests that need truncation because I am testing the whole Sidekiq stack and as a separate process Sidekiq needs to be able to see the records created in the original process. I want to keep the suite in general as :transaction though as the rest of the tests are fine with that strategy.

To do this I add come config to the rspec config:

config.before(:each) do |example|
  DatabaseCleaner.start

  if example.metadata[:truncate]
    DatabaseCleaner.strategy = :truncation
  else
    DatabaseCleaner.strategy = :transaction
  end
end

Then add metadata to the test:

it 'should doit', truncate: true do
  # create records then run the background worker
  # wait for the background worker to finish
end

The problem is that Sidekiq gets ActiveRecord not found errors, and sure enough, there are no rows available to on a separate database (postgres) connection:

connector_test=# select count(*) from contacts;
 count 
-------
     0
(1 row)

If I stick byebug in the test I can confirm that there are in fact records:

(byebug) Contact.count
3

but the strategey does seem to be truncation:

(byebug) DatabaseCleaner[:active_record].strategy.class
DatabaseCleaner::ActiveRecord::Truncation

(byebug) DatabaseCleaner[:active_record].strategy
#<DatabaseCleaner::ActiveRecord::Truncation:0x00007f95e600ff68 @only=nil, @tables_to_exclude=["schema_migrations"], @pre_count=nil, @reset_ids=nil, @cache_tables=true, @db=:default>

If I re-run the test after configuring truncation for all of the tests:

config.before(:suite) do
   DatabaseCleaner.strategy = :truncation
   
end

I can see that the strategy in the test is the same:

(byebug) DatabaseCleaner[:active_record].strategy
#<DatabaseCleaner::ActiveRecord::Truncation:0x00007f8faf0ccec0 @only=nil, @tables_to_exclude=["schema_migrations"], @pre_count=nil, @reset_ids=nil, @cache_tables=true, @db=:default>

and the records are available in the test:

(byebug) Contact.count
3

but this time the records are also available in the database:

connector_test=# select count(*) from contacts;
 count 
-------
     3
(1 row)

and my test passes. These are the versions of relevant gems I am using

$ bundle info database_cleaner
  * database_cleaner (1.7.0)
	Summary: Strategies for cleaning databases.  Can be used to ensure a clean state for testing.
	Homepage: https://github.com/DatabaseCleaner/database_cleaner
	Path: /Users/will/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/database_cleaner-1.7.0

$ bundle info rspec-rails
  * rspec-rails (3.8.3)
	Summary: RSpec for Rails
	Homepage: https://github.com/rspec/rspec-rails
	Path: /Users/will/.rbenv/versions/2.6.5/lib/ruby/gems/2.6.0/gems/rspec-rails-3.8.3

Upgrading database cleaner

So I noticed as I was gathering the version data that database_cleaner was out of date (I could have sworn that I updated it already!). Anyway, after upgrading database_cleaner it now looks like inside the test the reported strategy matches the observed behaviour:

(byebug) DatabaseCleaner[:active_record].strategy
#<DatabaseCleaner::ActiveRecord::Transaction:0x00007ff8ba938928 @db=:default>

This is despite my strategy switch seeming to work OK in the rspec config:

[57, 66] in /Users/will/clients/foo/repos/web-app/spec/rails_helper.rb
   57:   config.before(:each) do |example|
   58:     $redis.flushall
   59:     DatabaseCleaner.start
   60: 
   61:     byebug
=> 62:     if example.metadata[:truncate]
   63:       DatabaseCleaner.strategy = :truncation
   64:     else
   65:       DatabaseCleaner.strategy = :transaction
   66:     end
(byebug) example.metadata[:truncate]
true

and as the test is using transactions not truncation the external process can't see the records and the test fails. Any ideas what's going on here?

@botandrose
Copy link
Contributor

Hi, @wjessop , thanks for the issue! At a glance, my first guess is that the #clean command is being called before the #strategy= command. Can you confirm or refute this?

@botandrose
Copy link
Contributor

Fat fingered the Close and comment button instead of the Comment button 😅

@wjessop
Copy link
Author

wjessop commented May 22, 2020

Thanks for the reply @botandrose! I found some around that looked like it might be the culprit:

config.around(:each) do |example|
  if example.metadata[:bypass_cleaner]
    example.run
  else
    DatabaseCleaner.cleaning do
      example.run
    end
  end
end

I commented it out and the records are now available outside the process, and the test passes, but rather confusingly the strategy still reports as transactional:

(byebug) DatabaseCleaner[:active_record].strategy
#<DatabaseCleaner::ActiveRecord::Transaction:0x00007f8f78b8d300 @db=:default>

I've got Spring disabled BTW, just to eliminate that as a possible issue.

@aledalgrande
Copy link
Contributor

Had the same problem and removed the around block, problem solved. Instead of using the around block I use:

  config.before(:each) do
    DatabaseCleaner.start
  end

  config.append_after(:each) do
    DatabaseCleaner.clean
  end

skalar-bcawkwell added a commit to gramo-org/echo_common that referenced this issue Jan 15, 2021
…used

In some tests, we might ensure that a transaction has been rolled back (e.g. if
an error occurred). Since currently we do not support transactions within other
transactions, those tests use the `omit_database_transaction` flag which means
the test does not get wrapped in a transaction. The problem is that these tests
now leave data lying around after running.

Our original plan to solve this was to find a way to still rollback in the app
despite already running in a transaction. However Sequel does not support naming
a transaction from what I can tell, and anyway there is no way to pass args to
the transaction via the `DatabaseCleaner` gem. Going through some issues I found
this issue:
  DatabaseCleaner/database_cleaner#652

Basically this person wants to read data from the DB in another app, but in only
some tests. He took the approach of switching "strategy" to accomplish it. I am
basically stealing the same idea since it seems to work and was quite trivial.
The big downside is that those tests will probably take longer to run, luckily
only 5 tests use this flag currently in echo, so this should not be too big of a
problem.
skalar-bcawkwell added a commit to gramo-org/echo_common that referenced this issue Jan 15, 2021
…used (#109)

In some tests, we might ensure that a transaction has been rolled back (e.g. if
an error occurred). Since currently we do not support transactions within other
transactions, those tests use the `omit_database_transaction` flag which means
the test does not get wrapped in a transaction. The problem is that these tests
now leave data lying around after running.

Our original plan to solve this was to find a way to still rollback in the app
despite already running in a transaction. However Sequel does not support naming
a transaction from what I can tell, and anyway there is no way to pass args to
the transaction via the `DatabaseCleaner` gem. Going through some issues I found
this issue:
  DatabaseCleaner/database_cleaner#652

Basically this person wants to read data from the DB in another app, but in only
some tests. He took the approach of switching "strategy" to accomplish it. I am
basically stealing the same idea since it seems to work and was quite trivial.
The big downside is that those tests will probably take longer to run, luckily
only 5 tests use this flag currently in echo, so this should not be too big of a
problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants