Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix an issue where driven_by(:selenium) is always called #2188

Merged

Conversation

sinsoku
Copy link
Contributor

@sinsoku sinsoku commented Oct 16, 2019

Because the before process defined in SystemExampleGroup is executed
before the use defined before process, driven_by(:selenium) is always
executed.

This commit fixes the driver configuration to be lazy.

@JonRowe
Copy link
Member

JonRowe commented Oct 16, 2019

👋 Hi, thanks for this, unfortunately driven_by(:selenium) will always be called due to:

https://github.com/rails/rails/blob/cf43b71b64e21182c34e0907167e13fc917436bd/actionpack/lib/action_dispatch/system_test_case.rb#L160

If you want to prevent RSpec from calling it again, I'd accept a PR that allows configuring a default driver and using that default value, but I believe this needs to be not lazily loaded.

spec/rspec/rails/example/system_example_group_spec.rb Outdated Show resolved Hide resolved
Because the before process defined in SystemExampleGroup is executed
before the use defined before process, driven_by(:selenium) is always
executed.

This commit fixes to check the driver configuration at the end of
before hooks.
@sinsoku sinsoku force-pushed the fix_duplicate_configuration_for_driver branch from 41a607e to 850f450 Compare October 17, 2019 03:58
@sinsoku
Copy link
Contributor Author

sinsoku commented Oct 17, 2019

Thanks for your reviews.

I rewrote the code to check the driver at the end of before hooks.
Please review again.

@sinsoku
Copy link
Contributor Author

sinsoku commented Oct 17, 2019

I also created a pull request (rails/rails#37476) to make driven_by(:selenium) lazy.
If both rails and rspec-rails pull requests are merged, then calling driven_by(:selenium) can be prevented and rails/rails#37410 will be resolved.

@pirj pirj dismissed JonRowe’s stale review October 17, 2019 19:09

group.new.driver is no more

Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for asking to reconsider the implementation.
Wondering if like this approach and it works for your case.

@JonRowe JonRowe merged commit cbd4d02 into rspec:master Oct 18, 2019
JonRowe added a commit that referenced this pull request Oct 18, 2019
@sinsoku sinsoku deleted the fix_duplicate_configuration_for_driver branch October 19, 2019 05:13
JonRowe added a commit that referenced this pull request Jan 12, 2020
…_driver

Fix an issue where driven_by(:selenium) is always called
JonRowe added a commit that referenced this pull request Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants