Actually having re-reviewed the codebase. If you want to add a 1liner to include the extra file. I'm happy with that as a bug request fix. But altering the requires would be a feature change (which is fine), but I'd want that tweaking in other files ideally.

The reason being is that the one autoload file which "isn't" autoloaded is this file (For a reason I'm not sure of). Essentially a user could in theory use cucumber without either of rspec or minitest in place (Which I'm not sure is great)

For your specific use case (Both loaded), this probably needs some form of manual user input. So a user would load something like cucumber/rails/rspec for rspec and cucumber/rails/minitest for minitest? I don't know, what do you think is best? Given you are using this currently.

Also there's some argument it might be a major change (Again not a problem), but would need treating accordingly.

Whilst the file already has conditionals. Given these are procedural loaders I'd rather remove conditionals.

Maybe look into making another file for minitest?

We could do something to the inverse maybe to avoid the breaking change? Maybe something like: if minitest is loaded and RSpec is not, then dont require RSpec?

Sounds like a good idea.

So basically we have 4 situations.

• Neither (I don't like this situation, but for now we can tolerate it. I "believe" the suite would crash with a Minitest failure)
• RSpec (This works I believe)
• Minitest (This one I believe works, beause you would fail the first require, and then work on the 2nd require)
• Both (This is the situation you have, which is awkward I admit)

Atm we load RSpec when only RSpec is present (:green_book:)
The rest are all possibly subject to change / tweaks (:green_book:)

Anything you can do RE that is good in my eyes. If you wanted to check if these were already pre-loaded we could. However I don't know what users allow delegate their loading to us. i.e. it could be people just require 'cucumber/rails/rspec'

I know I've probably opened up a can of worms here. So I'll apologise for that. Even the name of the file gives away we perhaps are a bit behind the times here.

@luke-hill Would that translate to: change the else to another if defined?(...) conditional around the second [Cucumber::Rails::World, ActionDispatch::Integration::Session].each do |klass| ?

Possibly. But I think a better approach would be to have a minitest initializer and an rspec one. Because at the moment, the rspec one (named as such), is controlling minitest because we (perhaps incorrectly), assumed that if one existed, the other didn't. And one always existed.

In the truth table, there's 4 situations. We need to cover each of them ideally. The OP did suggest we could cover 2 of the use cases (1 is loaded and the other isn't), by just checking for each of them

@jsmestad any thoughts on this?

@luke-hill I think the request became larger than what I was expecting and had to move onto other tasks. Since then we just switched to RSpec completely to avoid the issue altogether.

I agree with the approach, but the time involved was going to be too much for me to take on during year-end scramble.

I'll kill the PR @jsmestad but leave the issue open so someone else can come into it. I'm thinking this will be hard to fix in a regular PR, but we could make a brute force change and cut a 3.0 release and just say

"cucumber rails always had an assumption you would use rspec, but now you need to explicitly tell it you want to use rspec, or minitest"