DatabaseCleaner / database_cleaner-active_record

Strategies for cleaning databases using ActiveRecord. Can be used to ensure a clean state for testing.
MIT License
64 stars 63 forks source link

Update `truncate_tables` to allow argument list of tables #39

Closed sarahkemi closed 3 years ago

sarahkemi commented 4 years ago

( ⬇️ copied from https://github.com/DatabaseCleaner/database_cleaner/pull/598)

This PR is allows truncate_tables in the ActiveRecord adapter to take in a list of table names in two different ways: 1) as a regular array of tables: truncate_tables(['users','agents']) or 2) as a list of arguments: truncate_tables('users','agents'). This is because when using DatabaseCleaner in conjunction with a connection adapter to imitate the truncate_tables method as it's implemented in the rails 6 version of active record, cleaning with truncation is broken (as shown in an example error below).

An error occurred in a `before(:suite)` hook.
Failure/Error: DatabaseCleaner.clean_with(:truncation)

ActiveRecord::StatementInvalid:
  RandomAdapter::MysqlError: random_adapter_query_recv: 1103 Incorrect table name '["table1", "table2", "table3'
.
.
.
# /usr/local/bundle/gems/database_cleaner-1.7.0/lib/database_cleaner/active_record/truncation.rb:239:in `block in clean'
# /usr/local/bundle/gems/database_cleaner-1.7.0/lib/database_cleaner/active_record/truncation.rb:235:in `clean'
# /usr/local/bundle/gems/database_cleaner-1.7.0/lib/database_cleaner/base.rb:48:in `clean_with'
# /usr/local/bundle/gems/database_cleaner-1.7.0/lib/database_cleaner/configuration.rb:91:in `block in clean_with'
# /usr/local/bundle/gems/database_cleaner-1.7.0/lib/database_cleaner/configuration.rb:91:in `each'
# /usr/local/bundle/gems/database_cleaner-1.7.0/lib/database_cleaner/configuration.rb:91:in `clean_with'
# ./spec/rails_helper.rb:47:in `block (2 levels) in <top (required)>'

As you can see here, the truncate_tables method is trying to the array of tables as one table name.

Let me know your thoughts on this! Thanks!

codecov[bot] commented 4 years ago

Codecov Report

Merging #39 into master will increase coverage by 0.07%. The diff coverage is 95.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
+ Coverage   90.03%   90.11%   +0.07%     
==========================================
  Files          11       11              
  Lines         512      526      +14     
==========================================
+ Hits          461      474      +13     
- Misses         51       52       +1     
Impacted Files Coverage Δ
lib/database_cleaner/active_record/truncation.rb 88.73% <90.90%> (-0.40%) :arrow_down:
.../database_cleaner/active_record/truncation_spec.rb 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 25f6889...4d6a0ae. Read the comment docs.

botandrose commented 4 years ago

@sarahkemi Thanks for the PR! Now that we've split the database_cleaner gem into many adapter gems, one for each ORM, we can finally test against modern versions, including Rails 6.0. Right now, If you view the Travis CI matrix, all the tests are passing on Rails 6.0, so as far as I can tell, this should work with Rails 6.0 today!

Looking at the backtrace of the error you posted, I can see that you are using an old v1.7.0 version of database_cleaner, and v1.8.4 is the current version. Can you try updating your app to v1.8.4, and see if that fixes the error, or changes the error to something else?

sarahkemi commented 4 years ago

@botandrose looks like this PR might be ready to go! 😄

botandrose commented 4 years ago

Hi @sarahkemi! Great job on getting this PR green, and thank you for your continued efforts on it, especially during the gem extraction period of 1.8 -> 2.0 that I'm guessing has caused you to perform extra work, so I appreciate it.

I'd like to merge this, but I'm needing to see a test that demonstrates the concrete bug that we're fixing here. Currently, the tests in this PR seem to only be verifying that the changes made to the code have indeed been made. Not that these changes fix something in the real-world usage of this gem. In other words, I want to see a test that reproduces the original bug that you encountered all the way back at the top, so that we can be sure of two things. Firstly, that the bug is actually fixed, and secondly, that this change is effectively addressing that bug.

Or, at least, that's what I'm thinking right now. What are your thoughts about this?

woto commented 4 years ago

Hi @botandrose here is the stack trace.

https://github.com/rails/rails/blob/v6.0.3.4/activerecord/lib/active_record/tasks/database_tasks.rb#L221 https://github.com/DatabaseCleaner/database_cleaner-active_record/blob/v2.0.0.beta/lib/database_cleaner/active_record/truncation.rb#L156

/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/database_cleaner-active_record-2.0.0.beta/lib/database_cleaner/active_record/truncation.rb:156:in `truncate_tables'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.4/lib/active_record/tasks/database_tasks.rb:221:in `block in truncate_tables'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.4/lib/active_record/connection_handling.rb:251:in `swap_connection_handler'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.4/lib/active_record/connection_handling.rb:159:in `with_handler'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.4/lib/active_record/connection_handling.rb:117:in `connected_to'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.4/lib/active_record/tasks/database_tasks.rb:213:in `truncate_tables'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.4/lib/active_record/tasks/database_tasks.rb:228:in `block in truncate_all'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.4/lib/active_record/tasks/database_tasks.rb:227:in `each'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.4/lib/active_record/tasks/database_tasks.rb:227:in `truncate_all'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-6.0.3.4/lib/active_record/railties/databases.rake:73:in `block (2 levels) in <main>'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/task.rb:281:in `block in execute'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/task.rb:281:in `each'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/task.rb:281:in `execute'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/task.rb:219:in `block in invoke_with_call_chain'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/task.rb:199:in `synchronize'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/task.rb:199:in `invoke_with_call_chain'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/task.rb:188:in `invoke'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/application.rb:160:in `invoke_task'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/application.rb:116:in `block (2 levels) in top_level'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/application.rb:116:in `each'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/application.rb:116:in `block in top_level'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/application.rb:125:in `run_with_threads'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/application.rb:110:in `top_level'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/application.rb:83:in `block in run'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/application.rb:186:in `standard_exception_handling'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/lib/rake/application.rb:80:in `run'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rake-13.0.1/exe/rake:27:in `<top (required)>'
/Users/r.kornev/.rbenv/versions/2.7.2/bin/rake:23:in `load'
/Users/r.kornev/.rbenv/versions/2.7.2/bin/rake:23:in `<top (required)>'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/cli/exec.rb:63:in `load'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/cli/exec.rb:63:in `kernel_load'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/cli/exec.rb:28:in `run'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/cli.rb:476:in `exec'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor.rb:399:in `dispatch'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/cli.rb:30:in `dispatch'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor/base.rb:476:in `start'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/cli.rb:24:in `start'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/libexec/bundle:46:in `block in <top (required)>'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/2.7.0/bundler/friendly_errors.rb:123:in `with_friendly_errors'
/Users/r.kornev/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/libexec/bundle:34:in `<top (required)>'
/Users/r.kornev/.rbenv/versions/2.7.2/bin/bundle:23:in `load'
/Users/r.kornev/.rbenv/versions/2.7.2/bin/bundle:23:in `<main>'
Tasks: TOP => db:truncate_all
botandrose commented 4 years ago

@sarahkemi @woto Ahhh I see the issue now. It looks like this was due to DatabaseCleaner monkeypatching the internals of the AR adapters. This has been fixed in master... see #33 for more details. Can you switch to using the master branch and confirm/deny that it fixes this for you?

woto commented 4 years ago

@botandrose Yes it works in master. Thanks!

botandrose commented 3 years ago

Closing, since I think we're working on Rails 6 now. Feel free to reopen if there are still issues.