Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Default cucumber rails database_cleaner hook is suboptimal #215

Closed
phuongnd08 opened this Issue · 12 comments

9 participants

@phuongnd08

The default cucumber rails database_cleaner hook make a call to DatabaseCleaner.start, which in some cases will cause the transaction mode to begin. Under that condition, the test is unstable as it raises "This connection is still waiting for a result" or "Mysql2::Error: SAVEPOINT active_record_1 does not exist" (This is a very notorious problem happen with the combination of Cucumber + Capybara + Mysql)

Developer should be able to explicitly change the DatabaseCleaner strategy before calling DatabaseCleaner.start, which may be impossible because the shipped hook may run first. So, the hook should be removed instead

@aslakhellesoy

If you don't want the hook to run, just tag your scenario with @no-database-cleaner

Or did I misunderstand what you're trying to achieve?

P.S. Your emotions don't add value to the conversation

@phuongnd08

What should I do if I need all of my features to behave as if they has this '@no-database-cleaner'? I think the hook should be removed instead. It's too magical for me as of now.

P.S. Sorry for being uncool. Some time banging your head against wall is not so fun.

@aslakhellesoy

It's not clear to me what you're trying to accomplish. Not run DBcleaner at all for all scenarios? Always run, but use different strategies? You can do that with another hook.

You might want to start a thread on the mailing list to get advice from more people

@phuongnd08

Okay, I rephrased the issue description.

@mdgreenfield

Was this thread taken to the mailing list? Here are my experiences with the database_cleaner and the default hooks.

I too have a need to run scenarios without calling database_cleaner. In my case it's just a few scenarios (each tagged as @alpha which I am actually running against our development database (calling RAILS_ENV=alpha).

I believe @phuongnd08 should be able to set DatabaseCleaner.strategy to nil to achieve what he wants (according to the database_cleaner docs). However, I noticed that setting this and running my tests results in the data still being deleted.

In my particular case, it's just the @alpha tags that I don't want to run database_cleaner on. However, the only way I was able to prevent database cleaner from being run was by tagging my scenarios with both @alpha and @no-database-cleaner. Ideally, I'd like to just be able to use the @alpha tag instead of having to specify both since I'm fearful that someone else on the project could potentially use the @alpha tag without @no-database-cleaner causing all the data on our development server to be lost.

As it currently is now, there is no way to disable or override the @no-database-cleaner tag (to my knowledge) which is why I think that the default database_cleaner tags should be removed.

@dpritchett

The fact that rake cucumber throws a You need to add database_cleaner to your Gemfile (in the :test group) if you wish to use it. error after initial setup is awkward behavior. I would prefer to either have database_cleaner as a prerequisite for cucumber-rails (thus making the error go away) or to have this code path be disabled by default.

I recognize that most users will probably not be using sqlite and thus won't ever see this error, but it's a curious failure nonetheless.

Edit: FWIW I got here after bombing out on the initial rails-cucumber setup from page 282 of BDD In Rails.

Further edit: I checked the gemspec and database-cleaner is a dev dependency but not a test dependency. I put cucumber-rails back in my :development group and now I don't get the exception, instead I get the WARNING: Cucumber-rails required outside of env.rb. The rest of loading is being defered until env.rb is called.
To avoid this warning, move 'gem cucumber-rails' under only group :test in your Gemfile
warning which while worrisome doesn't prevent me from running tests.

Thanks!

@dsgh

@phuongnd08 Can't you just place some code like the following in env.rb, before the require 'cucumber/rails' line?

Before('@truncation') do
  DatabaseCleaner.strategy = :truncation
end

Before('~@truncation') do
  DatabaseCleaner.strategy = :transaction
end

This will change the strategy before the cucumber-rails hook calls DatabaseCleaner.start, I think.

@mattwynne
Owner

I'm just catching up on this. If I understand correctly, a minority of cucumber-rails users would prefer for cucumber/rails/hooks/database_cleaner.rb not to be required by default when cucumber-rails boots?

Maybe we should add some kind of configuration block to allow you to opt out of the hook loading?

@mattscilipoti

@mattwynne That sounds like a great solution.

I prefer that the database is not cleaned up after each run. It is very helpful in debugging. Also, I sometimes use those wonderful cucumber steps to setup demo data in my development database. Technically, we just need too make sure the system is in correct state before starting a test. Unfortunately, the @no-database-cleaner tag disables DatabaseCleaner completely.

Is there a way to override the After('~@no-database-cleaner')?

@urbanautomaton

I'm just catching up on this. If I understand correctly, a minority of cucumber-rails users would prefer for cucumber/rails/hooks/database_cleaner.rb not to be required by default when cucumber-rails boots?

I think having the option to prevent this would definitely be nice. Adding @no-database-cleaner works fine, but it feels a bit counter-intuitive to have to use it on scenarios where I am running database_cleaner, just in a different way.

Totally understand if this is low priority though, given the existence of a workaround. I'd be happy to submit a PR if you'd be interested (and could give a pointer or two in terms of how/where you'd like it done).

Cheers,
Simon

@mattwynne
Owner

Thanks @urbanautomaton - a pull request is definitely the way forward here. All I'd ask is that you try to include tests in your patch, but I'll give you some help with that if you're not sure how to do it. Once we have some code we'll have something more concrete to discuss.

@urbanautomaton urbanautomaton referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@urbanautomaton urbanautomaton referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@urbanautomaton urbanautomaton referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@urbanautomaton urbanautomaton referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@urbanautomaton urbanautomaton referenced this issue from a commit in urbanautomaton/cucumber-rails
@urbanautomaton urbanautomaton Allow users to disable database_cleaner hooks
The hooks that automatically invoke database_cleaner before every
scenario can sometimes cause problems when switching cleaning strategies
between scenarios (issue #215).

To allow a user to take full control of when database_cleaner is
invoked, the configuration option
Cucumber::Rails::Database.autorun_database_cleaner is provided,
defaulting to true.
dbe2091
@meismann meismann referenced this issue from a commit
@meismann meismann Describe configuration option 'autorun_database_cleaner' in README
Issues #215 and #232 resulted in adding a configuration option to disable the by-default-usage of DatabaseCleaner:

Cucumber::Rails::Database.autorun_database_cleaner = false

This has to go into the README so that users can easily learn about it. Currently Google finds the related issues first, which is not desirable either.
6616330
@meismann meismann referenced this issue from a commit
@meismann meismann Describe configuration option 'autorun_database_cleaner' in README
Issues #215 and #232 resulted in adding a configuration option to
disable the by-default-usage of DatabaseCleaner:

Cucumber::Rails::Database.autorun_database_cleaner = false

This has to go into the README so that users can easily learn about
it. Currently Google finds the related issues first, which is not
optimal.
1ee06b4
@Kosmas
Owner

Closing old issues. If they are still relevant, please reopen

@Kosmas Kosmas closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.