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

Omit webdrivers gem from Gemfile template #48847

Merged

Conversation

seanpdoyle
Copy link
Contributor

@seanpdoyle seanpdoyle commented Jul 29, 2023

Motivation / Background

As of Selenium 4.6, the Selenium Manager is capable of managing Chrome Driver installations and integrations. As of Selenium 4.11, the
Selenium Manager is capable of capable of resolving the Chrome for
Testing installation
path.

Detail

By omitting the gem declaration from the Gemfile.tt, newly generated applications and applications updating their Gemfile in lockstep with newer Rails versions can shed the dependency and avoid test failures introduced by newly released Chrome versions (like, for example, titusfortner/webdrivers#247).

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@simi
Copy link
Contributor

simi commented Jul 29, 2023

Wouild it make sense to ensure selenium-webdriver 4.6+ gem is installed in Gemfile?

@seanpdoyle seanpdoyle force-pushed the omit-webdrivers-from-gemfile-template branch 2 times, most recently from 486bb46 to a14b518 Compare July 29, 2023 11:29
@seanpdoyle
Copy link
Contributor Author

Wouild it make sense to ensure selenium-webdriver 4.6+ gem is installed in Gemfile?

That's a great idea. I've changed both Rails' development Gemfile (the one you've linked to here) as well as the Gemfile.tt template file to specify >= 4.6.0.

@titusfortner
Copy link
Contributor

It should be Selenium 4.11 (not yet released) because while 4.6 - 4.10 manages drivers, it does not support the new Chrome for Testing location for chromedriver 115+.

@seanpdoyle
Copy link
Contributor Author

@titusfortner thank you for providing that context!

I'd like to note that CI was passing when locking to version 4.6, but will fail indefinitely until Selenium releases 4.11.

I'm now subscribed for those releases, so I'll come back to ping this PR once that occurs (@titusfortner I'll be you're also patiently watching and waiting, so don't hesitate to ping this PR if you beat me to it).

@seanpdoyle seanpdoyle force-pushed the omit-webdrivers-from-gemfile-template branch from c25a627 to 08aa7a5 Compare July 31, 2023 20:21
@seanpdoyle
Copy link
Contributor Author

@titusfortner there is a 4.11 Selenium release. I've bundle installed and force-pushed this branch to kick off CI.

@guilleiguaran
Copy link
Member

Looks like we need to use a lower version for Ruby 2.7:

Could not find compatible versions
--
  |  
  | Because selenium-webdriver >= 4.9.1 depends on Ruby >= 3.0
  | and Gemfile depends on selenium-webdriver >= 4.11.0,
  | Ruby >= 3.0 is required.
  | So, because current Ruby version is = 2.7.8,
  | version solving has failed.

@seanpdoyle
Copy link
Contributor Author

seanpdoyle commented Jul 31, 2023

@guilleiguaran thank you for sharing that, that's good information! That's unfortunate that there is a Ruby version requirement attached to Selenium's Ruby bindings.

It's probably a discussion worth having outside of this branch, but Ruby 2.7 is end-of-life as of 2023-03-31.

Since these changes are focused on template files for new applications' test suites, I wonder if there's a way to work around Rails' support for 2.7.

Gemfile Outdated Show resolved Hide resolved
actionpack/lib/action_dispatch/system_testing/browser.rb Outdated Show resolved Hide resolved
@titusfortner
Copy link
Contributor

Selenium supports what Ruby supports, and I'll strongly defend that as a feature and not a bug for any and all Ruby open source projects.

Alternately, webdrivers 5.3 now has support for chromedriver 115+ and still works for Ruby 2.7
The downside is that it restricts Selenium 4.11+ and gives a post_install_message
That might be better at this point, though?

As of Selenium 4.6, [the Selenium Manager is capable of managing Chrome
Driver installations and integrations][readme]. As of Selenium 4.11, the
Selenium Manager is capable of [capable of resolving the Chrome for
Testing installation][] path.

By omitting the `gem` declaration from the `Gemfile.tt`, newly generated
applications and applications updating their `Gemfile` in lockstep with
newer Rails versions can shed the dependency and avoid test failures
introduced by newly released Chrome versions (like, for example,
[titusfortner/webdrivers#247][]).

[readme]: https://github.com/titusfortner/webdrivers/tree/43f8ac436cc4121c903c1c611dfe76088ef6cbab#update-selenium-manager
[titusfortner/webdrivers#247]: titusfortner/webdrivers#247
[capable of resolving the Chrome for Testing installation]: rails#48847 (comment)

Co-authored-by: Titus Fortner <titusfortner@users.noreply.github.com>
@seanpdoyle seanpdoyle force-pushed the omit-webdrivers-from-gemfile-template branch from 3ba0342 to 9a53234 Compare August 1, 2023 13:22
@rafaelfranca
Copy link
Member

We can't drop support to old rubies without bumping the major version of Rails. We are not going to bump the major version of Rails for the next release so we need to support Ruby 2.7.

@seanpdoyle seanpdoyle force-pushed the omit-webdrivers-from-gemfile-template branch from 5e74c04 to 57ab84b Compare August 2, 2023 13:57
@seanpdoyle seanpdoyle force-pushed the omit-webdrivers-from-gemfile-template branch from 57ab84b to 425f100 Compare August 2, 2023 15:20
@rafaelfranca rafaelfranca merged commit e980f15 into rails:main Aug 2, 2023
9 checks passed
@seanpdoyle seanpdoyle deleted the omit-webdrivers-from-gemfile-template branch August 2, 2023 18:00
Eric-Guo added a commit to Eric-Guo/product_hunt that referenced this pull request Aug 3, 2023
Eric-Guo added a commit to thape-cn/web that referenced this pull request Aug 3, 2023
Eric-Guo added a commit to thape-cn/oauth2id that referenced this pull request Aug 3, 2023
Eric-Guo added a commit to thape-cn/previous_website that referenced this pull request Aug 14, 2023
Eric-Guo added a commit to Eric-Guo/coreui4-rails-starter that referenced this pull request Aug 19, 2023
paulreece pushed a commit to paulreece/rails-paulreece that referenced this pull request Aug 26, 2023
As of Selenium 4.6, [the Selenium Manager is capable of managing Chrome
Driver installations and integrations][readme]. As of Selenium 4.11, the
Selenium Manager is capable of [capable of resolving the Chrome for
Testing installation][] path.

By omitting the `gem` declaration from the `Gemfile.tt`, newly generated
applications and applications updating their `Gemfile` in lockstep with
newer Rails versions can shed the dependency and avoid test failures
introduced by newly released Chrome versions (like, for example,
[titusfortner/webdrivers#247][]).

[readme]: https://github.com/titusfortner/webdrivers/tree/43f8ac436cc4121c903c1c611dfe76088ef6cbab#update-selenium-manager
[titusfortner/webdrivers#247]: titusfortner/webdrivers#247
[capable of resolving the Chrome for Testing installation]: rails#48847 (comment)

Co-authored-by: Titus Fortner <titusfortner@users.noreply.github.com>
kamipo added a commit that referenced this pull request Sep 6, 2023
Follow-up to #48847.

This was originally "The webdrivers gem uses this proc to update web
drivers.".
koic added a commit to koic/rails that referenced this pull request Sep 13, 2023
## Motivation / Background

Continuous Integrating rubocop-rails_config after upgrading from Rails 7.0.7.2 to Rails 7.0.8 now fails:

```console
(snip)
rubocop --except=Style/StringLiterals,Style/FrozenStringLiteralComment .
cp ./test/fixture/.rubocop.yml rails_test/.rubocop.yml
cd rails_test
Inspecting 28 files
C...........................

Offenses:

Gemfile:71:1: C: [Correctable] Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end.

28 files inspected, 1 offense detected, 1 offense autocorrectable
rake aborted!
```

https://github.com/toshimaru/rubocop-rails_config/actions/runs/6167995304/job/16739825926

Upon checking, redundant blank was added from version 7.0.8. This was `gem "webdrivers"` before 7.0.7.2.

### Rails 7.0.7.2

```console
$ ruby -v
ruby 3.3.0dev (2023-09-08T16:09:30Z master af5df9ee5e) [x86_64-darwin22]

$ cat Gemfile
# frozen_string_literal: true

source "https://rubygems.org"

gem "rails", "7.0.7.2"

$ bundle install
(snip)

$ bundle exec rails new example && cd $_
(snip)

% tail -n5 Gemfile
  # Use system testing [https://guides.rubyonrails.org/testing.html#system-testing]
  gem "capybara"
  gem "selenium-webdriver"
  gem "webdrivers"
end
```

### Rails 7.0.8

```console
$ cat Gemfile
# frozen_string_literal: true

source "https://rubygems.org"

gem "rails", "7.0.8"

$ bundle install
(snip)

$ bundle exec rails new example && cd $_
(snip)

% tail -n5 Gemfile
  # Use system testing [https://guides.rubyonrails.org/testing.html#system-testing]
  gem "capybara"
  gem "selenium-webdriver"

end
```

This is reproduced with Rails 7.0.8, Rails 7.1.0.beta1 and the main branch.

I understand this might not be of utmost importance, but I would appreciate
it if it could be backported to the Rails 7.0 branch, if possible.

## Detail

The blank line is filled in by rails#48847.
This PR removes the redundant blank line from Gemfile of new app.
Eric-Guo added a commit to Eric-Guo/tailwindcss-jit-stimulus that referenced this pull request Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants