Breaks with database_cleaner 2.0

mvz commented 3 years ago

Summary

When upgrading from database_cleaner 1.8 to 2.0,

Expected Behavior

cucumber-rails works in combination with database_cleaner 2.0.

Current Behavior

I get the following error when a @javascript scenario is started:

undefined method `strategy' for nil:NilClass (NoMethodError)

This seems to be coming from https://github.com/cucumber/cucumber-rails/blob/master/lib/cucumber/rails/database/strategy.rb#L11.

Possible Solution

I see two options:

Figure out where database_cleaner 2.0 stores the current strategy, if not in DatabaseCleaner.cleaners.

Use two configuration options instead of one, and create Before blocks like so:

Before '@javascript' do
DatabaseCleaner.strategy = Cucumber::Rails::Database.javascript_strategy
end

Before 'not @javascript' do
DatabaseCleaner.strategy = Cucumber::Rails::Database.plain_strategy
end

The latter option avoids having to use non-documented DatabaseCleaner APIs yet again.

Steps to Reproduce (for bugs)

Have a Rails app with javascript scenarios, cucumber-rails, and database_cleaner 1.8.
Upgrade database_cleaner to 2.0
Run the cukes

Context & Motivation

This came up when Dependabot tried to upgrade database_cleaner in my work projects.

Your Environment

Version used: 2.2.0
Operating System and version: Debian, unstable
Link to your project: N/A

luke-hill commented 3 years ago

ping @cgriego / @deivid-rodriguez Any thoughts on this? I know both of you worked on this small piece of code recently.

I'm happy if we don't have a quick fix just to put a custom error in so we know we don't support DB cleaner 2 (Which as of writing is brand new, so I don't view it as a negative just yet).

patrickmcgraw commented 3 years ago

I just ran into this problem using database_cleaner 1.8.5. The conditional seems to target >= 1.8.0.beta. I think just adding better messaging would be enough. Once I found where the message came from it was easy enough to figure out that database_cleaner did not have a strategy set before cucumber-rails tried to read from it. But figuring out where that message came from took a little while.

mvz commented 3 years ago

@patrickmcgraw your problem is probably different from mine, since everything runs fine for me with database_cleaner 1.8.5.

patrickmcgraw commented 3 years ago

Ah, fair point. Looks like they don't automatically require any of the cleaners anymore. Adding require 'database_cleaner/active_record' fixed it for me when I bumped to 2.0.0.

deivid-rodriguez commented 3 years ago

I checked the library (activeadmin) where I'm using this library and database_cleaner and everything seems up to date and we got no issues. Sorry I can't be of help.

mvz commented 3 years ago

@deivid-rodriguez that's actually very useful to know since now I can compare activeadmin's setup with mine.

deivid-rodriguez commented 3 years ago

Great! Something to be noted is that we don't require the full cucumber-rails, just the specific parts that we need. Maybe that's why we didn't hit it :man_shrugging:.

mvz commented 3 years ago

I compared ActiveAdmin and my own project and it turns out that the difference is that my project has require: false for database_cleaner, and ActiveAdmin doesn't. With database_cleaner version 1 this is no problems since cucumber-rails will do require 'database_cleaner' anyway. However, with database_cleaner version 2 it will do require 'database_cleaner/core' and therefore not load the ActiveRecord adapters.

The best fix for cucumber-rails is probably to check for DatabaseCleaner.cleaners being nil and emit a sensible warning in that case.

cucumber / cucumber-rails