-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add Rails 7.1 builds #2692
Add Rails 7.1 builds #2692
Conversation
For Rails 6.1 builds: webdrivers 5.3.1 is installed for Ruby 2.6/2.7 and the build is green, but webdrivers 5.2.0 for Ruby 3.0/3.1 and those builds fail. Problems are due to this:
Where "older version" is 5.2.0, and "cannot upgrade" only affects Ruby 2.5, where webdrivers 4.x is installed. At a glance, I can't see why 5.3.1 is not installed for all 2.6+ Ruby versions. |
It is dependency of a selenium-webdriver gem. For Ruby 3.0+, it installs selenium-webdriver v4.13.1. https://github.com/SeleniumHQ/selenium/blob/selenium-4.13.1-ruby/rb/selenium-webdriver.gemspec#L33 For Ruby 2.6/2.7, it installs selenium-webdriver v4.9.0, because selenium-webdriver v4.10.0+ required ruby version 3.0+. https://github.com/SeleniumHQ/selenium/blob/selenium-4.9.0/rb/selenium-webdriver.gemspec#L33 |
I think it's better to remove the webdrivers gem dependency in Ruby 3.0+. # https://github.com/rspec/rspec-rails/blob/main/example_app_generator/generate_app.rb#L34
- gsub_file "Gemfile", /.*chromedriver-helper.*/, "gem 'webdrivers'"
+ if RUBY_VERSION < "3.0"
+ gsub_file "Gemfile", /.*chromedriver-helper.*/, "gem 'webdrivers'"
+ end Because Rails omit webdrivers gem from Gemfile template in Ruby 3.0+. |
5809669
to
2d06e09
Compare
I suggest skipping features on Ruby 2.5 |
Only Rails 7 dropped webdriver, we still need it on 6.1 @pirj, also would you mind making your changes on your own branch / PR either to this branch or seperate. |
d0e3089
to
1a8cc68
Compare
@jcoyne 's webdriver removal actually weirdly works on 6.1 so I've picked that across with the skips for the broken specs which I'll work on seperately later to see if we can get closer to passing hre. |
@chaadow This PR is about getting our CI up and running, please use the mailing list if you have problems you want to discuss or open a bug report if you think you've found an issue but remember that rspec-rails is a thin wrapper around Rails test helpers, if they don't recognise a route anymore you'll need to change. |
@JonRowe yes, when i updated from rspec-rails 6.0.0 to 6.0.3, the issue was solved thanks! |
It was fixed here bd2bb24 Also I thought since the builds were failing here and i'm in the process of upgrading to rails 7.1 and had issues with rspec, that maybe the issue I have might be related to rails 7.1 👍🏼 |
7.1 did. rails/rails@9a53234 It’s not that Rails 7.1 dropped webdriver and we are forced to use webdrivers for rails 6.1/7.0. I’m unaware of a reliable solution for Rails 6.1 running on Ruby 2.5 to run browser specs. We have to follow the same practical approach in our test suite. I believe that my commit fixed features for Rails 6.1/7.0 running on Ruby 2.6+. I’ll be happy to send a separate PR, or @jcoyne you probably could accomodate those changes? I’m from my phone this weekend. |
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.
LGTM!
Please disregard my prior message.
Let’s deal with 7.1 failures and broken mailer specs separately?
Thanks!
8a1d3ee
to
7fcb9c8
Compare
The fixes for other builds has been merged as #2700 I'm continuing work on the 7.1 build here |
2dde28c
to
cf7d3a2
Compare
5b5f9aa
to
39d9fa0
Compare
39d9fa0
to
b4227ab
Compare
Final failure was a zeitwerk exception with our generator, I've surpressed this by ignoring the file after much faffing around trying to fix the issue, it doesn't happen locally for me (but did show up in zeitwerks check) so this was a pain to debug (and is hidden in the builds by the frozen array non error). |
No description provided.