cucumber/cucumber-rails

handling of @javascript tag is confusing

akostadinov opened this issue · 11 comments

Presently handling of scenarios tagged with @javascript and without is hardcoded. This is very confusing, see #166.

I see no clear documentation when a scenario is supposed to be tagged @javascript and then it shouldn't. I believe that it will be much clearer if tag is named @shared-connection as well create ability to people to list tags that should be handled one way or another.

In current implementation if project wants to use @tag1, @tag2 and @tag3 with shared connection transactional clean-up and @tag4, @tag5 and @tag6 with non-shared connection truncation clean-up, then there seems to be no way of doing this.

For example if one has this in support/env.rb:

non_transactional_tags = %w[
  @tag4
  @tag5
  @tag6
].freeze

transactional = non_transactional.map {|t| "not #{t}" }

Before(*transactional) do
  Cucumber::Rails::Database.javascript_strategy = :transaction
  Cucumber::Rails::Database.before_js if Cucumber::Rails::Database.autorun_database_cleaner
end

Before non_transactional.join(' or ') do
  Cucumber::Rails::Database.javascript_strategy = :truncation
  Cucumber::Rails::Database.before_non_js if Cucumber::Rails::Database.autorun_database_cleaner
end

require 'cucumber/rails'

This doesn't help because default hooks will call Cucumber::Rails::Database.before_non_js for anything not tagged @javascript even though we ourselves called before_js. And if we move require 'cucumber/rails' on top, then database cleaner will already be started so our hooks will be pointless.

Hello :)

The JavaScript tag related to database strategy is documented here: https://github.com/cucumber/cucumber-rails/blob/main/features/choose_javascript_database_strategy.feature

Does that help?

I found this feature yesterday and it somehow helped understand what is going on, although I didn't really until I read the database strategy hooks and classes. The bigger issue is that one can't configure this behavior easily depending on project tags. I'm trying to workaround this with a custom strategy class now and will post what I come up with here if this may serve as a better example of what I'm trying to do.

Once you have a custom strategy defined then you can just leverage that inside whatever logic you want inside your hooks? I'm confused with where the issue lies?

Hi again. The issue is that there is special handling of @javascript tag that goes into the way when project wants to switch between strategies based on tags. Additionally @javascript inside project may mean a very different thing from what cucumber-rails intended. Probably issue for old project mainly. For example in the project I'm working on, that tag means to use an external browser instead of rake.

To avoid the special @javascript handling I had to create a null strategy and then just set DatabaseCleaner.strategy = .... 3scale/porta#2619

IMO there should be a simpler way to tell cucumber-rails to avoid @javascript special handling. I don't know. Now things are clear to me and I have a reasonable implementation, I don't care so much anymore.

If I had to design this feature, I'd let users configure behavior like:

Cucumber::Rails::Database.default_strategy = :transaction_non_shared_connection
Cucumber::Rails::Database.truncation_tags = %w[@no-txn @tag1]
Cucumber::Rails::Database.transaction_shared_connection_tags = %w[@javascript @tag2]
Cucumber::Rails::Database.transaction_non_shared_connection_tags = %w[@tag3 @tag4]

Where last line is redundant provided the default but if default was set to something else, it would have been useful.

Some of those articles are severely old / some of the points made are old. But to me now it feels like a "configure it how you wish" approach.

The custom strategy allows for just that, configure it how you wish. If the default tags don't serve their purpose for you, feel free to use others.

I believe (But I could be wrong), that those tags were there to express intent for what things should do in the js/non-js scenarios when executed in sep threads to the app proper. But again I could be wrong here. This all feels like behaviour that encompasses the default/s easily?

If you're happy with a workaround / think the defaults could be improved, feel free to submit a PR. But I'm not sure I'd class it as a bugfix. It would be more of a breaking change (Which I suppose is valid).

What could be interesting at least would be to improve documentation regarding those points in the README

To me besides some documentation, it should be easier and more obvious how to turn off the @javascript hack/feature.

I'm a bit confused, the only way this feature works is if you specifically use it by tagging it as a javascript. or am I missing some point. If you don't want to use it, just don't tag your scenarios as such?

The issue is that feature kicks in when a test is tagged with @javascript without apparent indication to user. User can tag scenario like that without being apparent that something will happen under the hood. There is nothing in README.md abour @javascript tag. So people can use this tag for whatever reason, not because they wanted to use the feature.

At the very least, something like this can be available:

Cucumber::Rails::Database.javascript_strategy = :null

Right 100% on board with you now. Sorry for the confusion. Yes we shouldn't amend any of our code then, just maybe highlight to users that we pre-determine certain things. That's deliberate as we want to make this project work out of the box for a lot of users.

Closing as fixed in main