cucumber-rails icon indicating copy to clipboard operation
cucumber-rails copied to clipboard

Cucumber::Rails::Database.javascript_strategy is too simple

Open jherdman opened this issue 2 years ago • 12 comments

👓 What did you see?

When database_cleaner-redis AND database_cleaner-active_record are in play, I cannot correctly configure them for the Cucumber::Rails::Database.javascript_strategy. I'm forced to consolidate upon a single strategy.

✅ What did you expect to see?

I am able to set the correct strategy for any given database cleaner adapter I'm using.

📦 Which tool/library version are you using?

cucumber-rails v2.5.1

🔬 How could we reproduce it?

Steps to reproduce the behavior:

  1. Set up some Rails app with cucumber-rails, Redis, and database-cleaner with both database_cleaner-active_record and database_cleaner-redis
  2. Add the following to features/support/env.rb:
# Remove/comment out the lines below if your app doesn't have a database.
# For some databases (like MongoDB and CouchDB) you may need to use :truncation instead.
begin
  DatabaseCleaner[:active_record].strategy = :transaction
  DatabaseCleaner[:redis].strategy = :deletion
rescue NameError
  raise "You need to add database_cleaner to your Gemfile (in the :test group) if you wish to use it."
end

Before('@javascript') do
  DatabaseCleaner[:redis].strategy = :deletion
end
  1. Run your Cucumber suite
  2. See error:
Using the default profile...
@javascript
Feature: Some feature
  As an...
  I want to...
  So that....

  Background:                                                           # features/something.feature:8
      The 'truncation' strategy does not exist for the redis ORM!  Available strategies: deletion (DatabaseCleaner::UnknownStrategySpecified)

📚 Any additional context?


This text was originally generated from a template, then edited by hand. You can modify the template here.

jherdman avatar Apr 07 '22 20:04 jherdman

Reproduction https://github.com/jherdman/cucumber-rails-database-cleaner-repro. Simply running bin/cucumber after clone, and install will reproduce.

I think my ideal world would be the total elimination of the default JavaScript strategy. We already have the ability to do before/after/around hooks using tags for @javascript, why do we need anything more than that?

jherdman avatar Apr 08 '22 13:04 jherdman

Should anyone find this and need a work around you can simply "alias" the deletion strategy as truncation as follows:

module DatabaseCleaner
  module Redis
    Truncation = Deletion
  end
end

jherdman avatar Apr 08 '22 14:04 jherdman

Instead of removing the default strategy, how would you feel if instead the default was set to the new null strategy. Which does nothing?

This would be much easier to do (Yes it would still be a breaking change). But it would be a trivial task as opposed to a bigger task.

luke-hill avatar Apr 12 '22 09:04 luke-hill

Perhaps simply offering a null strategy in which one must configure the Before/After hooks would be less disruptive? This would resolve my issue.

jherdman avatar Apr 12 '22 14:04 jherdman

So we have a new null strategy. But this must be configured. However this still cannot be default set (We forcibly set the default as you are seeing).

What I'm proposing is not removing the forcible setting, but instead forcibly setting it to the NullStrategy. Which would be a trivial (breaking), change. This then in theory would not cause issues for yourself.

I can make a sample branch if you would prefer to trial out the behaviour?

luke-hill avatar Apr 12 '22 14:04 luke-hill

That'd be great!

jherdman avatar Apr 12 '22 14:04 jherdman

I'll get something up by the end of the week. there's no guarantees this will fix your issue but we can see.

We've been discussing doing a major release soon for some other small things, so the major release isn't really a concern.

luke-hill avatar Apr 12 '22 14:04 luke-hill

And I pushed this down the priority queue again :( I'll try get to this soon, or failing that if you do feel free.

luke-hill avatar Jun 28 '22 08:06 luke-hill

@jherdman given the lack of other commenting on here? I'm unsure if this is as important as originally mentioned.

Is this still an issue for you?

luke-hill avatar Oct 20 '23 16:10 luke-hill

This is no longer an issue for me. Please feel free to close this issue if you feel that it's resolved to your satisfaction.

jherdman avatar Oct 20 '23 22:10 jherdman

It's something I don't think I have enough knowledge of to make a breaking change on.

Is this no longer an issue because of the monkeypatch you have done? Or something else?

luke-hill avatar Oct 24 '23 11:10 luke-hill

I'm no longer working on the project where this caused the problems for me.

jherdman avatar Oct 24 '23 12:10 jherdman