-
Notifications
You must be signed in to change notification settings - Fork 74
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
🚧🥒 Reduce Capybara / Cucumber config #3911
base: master
Are you sure you want to change the base?
Conversation
features/support/capybara.rb
Outdated
end | ||
# Before '@javascript' do | ||
# Capybara.current_driver = DEFAULT_JS_DRIVER | ||
# end | ||
|
||
Before '@chrome' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe
Before '@chrome' do | |
Before '@foreground' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't apply this suggestion in #3947
I think currently we're used to @chrome
and know what it means, so I don't feel that we need to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, also, why assume @foreground
means @chrome
and not @firefox
?
|
||
# Needed because cucumber-rails requires capybara/cucumber | ||
# https://github.com/cucumber/cucumber-rails/blob/7b47bf1dda3368247bf2d45bcb17a224e80ec6fd/lib/cucumber/rails/capybara.rb#L3 | ||
# https://github.com/teamcapybara/capybara/blob/2.18.0/lib/capybara/cucumber.rb#L17-L19 | ||
Before '@javascript' do | ||
Capybara.current_driver = DEFAULT_JS_DRIVER | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember this @javascript
tag was heavily patched by us. I had some updates to upstream capybara/cucumber
but IIRC it required Ruby 3. So I can check about this now we have Ruby 3.1 merged.
a930d27
to
e185ba4
Compare
.circleci/config.yml
Outdated
@@ -91,7 +91,7 @@ attach-to-workspace: &attach-to-workspace | |||
at: . | |||
|
|||
system-builder-ruby31: &system-builder-ruby31 | |||
image: quay.io/3scale/system-builder:stream8-ruby3.1 | |||
image: quay.io/3scale/system-builder:stream8-ruby3.1-chrome126 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
127+ didn't work with --disable-gpu
? That's worrying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it does. I was trying to fix current Circleci failures by downgrading chromedriver, with no success.
3f03640
to
154f5c9
Compare
This PR is stale because it has not received activity for more than 30 days. Remove stale label or comment or this will be closed in 15 days. |
Remove session creation Revert to :prefer_exact capybara matching Remove default capybara values
Most of the suggestions here were applied as part of #3947 (I was pointed to this PR only after I had submitted my own 🙃) |
The capybara configuration has grown a lot recently and I believe we're building on top of an outdated setup. Capybara/Selenium offers default features that we can use.