Use headless driver for next Rails release by stevepolitodesign · Pull Request #2746 · rspec/rspec-rails

stevepolitodesign · 2024-03-16T00:07:00Z

In the next release of Rails, the default driver was switched from :chrome to :headless_chrome as see in: rails/rails#50512 This is to ensure the new ci template will "work out of the box".

However, this will not work with applications using rspec-rails, since it still defaults to :selenium. Instead, GitHub actions will fail with the following error:

Selenium::WebDriver::Error::SessionNotCreatedError:
            session not created: Chrome failed to start: exited normally.
              (session not created: DevToolsActivePort file doesn't exist)
              (The process started from chrome location /usr/bin/google-chrome is no longer running, so ChromeDriver is assuming that Chrome has crashed.)

I've created a demo application to highlight the problem, and test the proposed solution. The commit history aims to break everything out change by change, but I will highlight the important parts below.

This commit replicates the bug. Here's the corresponding failure.
Without changing the test, I simply update the Gemfile to use this branch, resulting in a passing test.

In the next release of Rails, the default driver was switched from `:chrome` to `:headless_chrome` as see in: rails/rails#50512 This is to ensure the new [ci template][] will "work out of the box". However, this will not work with applications using `rspec-rails`, since it still defaults to `:selenium`. Instead, GitHub actions will fail with the following error: ``` Selenium::WebDriver::Error::SessionNotCreatedError: session not created: Chrome failed to start: exited normally. (session not created: DevToolsActivePort file doesn't exist) (The process started from chrome location /usr/bin/google-chrome is no longer running, so ChromeDriver is assuming that Chrome has crashed.) ``` [ci template]: https://github.com/rails/rails/blob/main/railties/lib/rails/generators/rails/app/templates/github/ci.yml.tt

stevepolitodesign · 2024-03-16T00:13:38Z

Just pointing out that rails/rails#51289 will remove the test job altogether. However, I think it's still important we keep parity with Rails.

JonRowe · 2024-03-18T21:08:31Z

👋 thanks!, can you update the spec to match?

Rails `main` is [currently][] set to `7.2.0.alpha`. However, `ci.yml` uses `'main'` for the `RAILS_VERSION`. [currently]: https://github.com/rails/rails/blob/5ab13c5a7798c2a9f96bebb1c68285dfb842d4f9/RAILS_VERSION

stevepolitodesign · 2024-03-18T23:59:45Z

@JonRowe I made a change in f122744 in an attempt to fix CI, but Rails main is currently set to 7.2.0.alpha. However, ci.yml uses 'main' for the RAILS_VERSION.

stevepolitodesign · 2024-03-19T00:02:23Z

I think my imlpementation is correct, since CI is passing with my demo application. Does ci.yml need to change instead? Something like:

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index f54e46d7..92453045 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -39,10 +39,10 @@ jobs:
          # Edge Rails (?) builds >= 2.7
          - ruby: 3.2
            env:
-             RAILS_VERSION: 'main'
+             RAILS_VERSION: '~> 7.2.0'
          - ruby: 3.1
            env:
-             RAILS_VERSION: 'main'
+             RAILS_VERSION: '~> 7.2.0'
 
          # Rails 7.1 builds >= 2.7
          - ruby: 3.2

JonRowe · 2024-03-19T08:33:52Z

The version specified in our ci.yml triggers fetching Rails from git, it doesn't affect the version string of Rails exposed to rspec, your original check was correct but it changed the default and theres a spec that checks it: spec/rspec/rails/example/system_example_group_spec.rb:33 what I meant was to conditionally define that and add the second spec like you have.

This reverts commit f122744.

stevepolitodesign · 2024-03-19T09:55:41Z

@JonRowe thank you for the additional context. I was surprised by the original failure since I figured the default would not change because of the conditional in the code. Do I need to do something like this?

Is there a way for me to test this locally?

diff --git a/spec/rspec/rails/example/system_example_group_spec.rb b/spec/rspec/rails/example/system_example_group_spec.rb
index a9b9ff83..1a19d932 100644
--- a/spec/rspec/rails/example/system_example_group_spec.rb
+++ b/spec/rspec/rails/example/system_example_group_spec.rb
@@ -29,7 +29,7 @@ module RSpec::Rails
       end
     end
 
-    describe '#driver' do
+    describe '#driver', if: ::Rails::VERSION::STRING.to_f < 7.2 do
       it 'uses :selenium driver by default' do
         group = RSpec::Core::ExampleGroup.describe do
           include SystemExampleGroup

JonRowe · 2024-03-19T13:09:19Z

I was surprised by the original failure since I figured the default would not change because of the conditional in the code.

It failed our Rails main build because we check for the specific value which you changed

stevepolitodesign · 2024-03-19T15:24:26Z

@JonRowe let me know if my latest change is what you expected. My confusion lies in the fact that I would have expected there to be two specs:

The existing #driver spec the ensures itss set to :selenium
A new #driver spec ensuring it's set to :selenium_chrome_headless if ::Rails::VERSION::STRING.to_f >= 7.2

However, using #take_screenshot as an example, it looks like it's only tested once, and with the conditional.

JonRowe · 2024-03-19T21:43:48Z

Thanks!

Follow-up to #1156 Creates parity with Rails' decision to [use a headless driver by default][headless]. This will be fixed in an [upcoming release][rspec] of rspec-rails, but I felt it was important to capture here. Additionally, it ensures the `screen_size` is the same as what is set in Rails. Removes `webdrivers` dependency in favor of `selenium-webdriver`. This generator assumes the app was generated with the `--skip-test`, which means we need to add the `selenium-webdriver` and `capybara` gems. Updates `action_dispatch-testing-integration-capybara` dependency to the most recent tagged release in an effort to suppress Dependabot notifications. [headless]:drive://github.com/rails/rails/pull/50512 [rspec]: rspec/rspec-rails#2746

Follow-up to #1156 Creates parity with Rails' decision to [use a headless driver by default][headless]. This will be fixed in an [upcoming release][rspec] of rspec-rails, but I felt it was important to capture here. Additionally, it ensures the `screen_size` is the same as what is set in Rails. Removes `webdrivers` dependency in favor of `selenium-webdriver`. This generator assumes the app was generated with the `--skip-test`, which means we need to add the `selenium-webdriver` and `capybara` gems. Updates `action_dispatch-testing-integration-capybara` dependency to the most recent tagged release in an effort to suppress Dependabot notifications. Ensure all files under `spec/support` are loaded by uncommenting a line generated by the RSpec installation script. [headless]:drive://github.com/rails/rails/pull/50512 [rspec]: rspec/rspec-rails#2746

Follow-up to #1156 Creates parity with Rails' decision to [use a headless driver by default][headless]. This will be fixed in an [upcoming release][rspec] of rspec-rails, but I felt it was important to capture here. Additionally, it ensures the `screen_size` is the same as what is set in Rails. Removes `webdrivers` dependency in favor of `selenium-webdriver`. This generator assumes the app was generated with the `--skip-test`, which means we need to add the `selenium-webdriver` and `capybara` gems. Updates `action_dispatch-testing-integration-capybara` dependency to the most recent tagged release in an effort to suppress Dependabot notifications. Ensure all files under `spec/support` are loaded by uncommenting a line generated by the RSpec installation script. [headless]: drive://github.com/rails/rails/pull/50512 [rspec]: rspec/rspec-rails#2746

Follow-up to #1156 Creates parity with Rails' decision to [use a headless driver by default][headless]. This will be fixed in an [upcoming release][rspec] of rspec-rails, but I felt it was important to capture here. Additionally, it ensures the `screen_size` is the same as what is set in Rails. Removes `webdrivers` dependency in favor of `selenium-webdriver`. This generator assumes the app was generated with the `--skip-test`, which means we need to add the `selenium-webdriver` and `capybara` gems. Updates `action_dispatch-testing-integration-capybara` dependency to the most recent tagged release in an effort to suppress Dependabot notifications. Ensure all files under `spec/support` are loaded by uncommenting a line generated by the RSpec installation script. [headless]: rails/rails#50512 [rspec]: rspec/rspec-rails#2746

Follow-up to #1156 Creates parity with Rails' decision to [use a headless driver by default][headless]. This will be fixed in an [upcoming release][rspec] of rspec-rails, but I felt it was important to capture here. Additionally, it ensures the `screen_size` is the same as what is set in Rails. Removes `webdrivers` dependency in favor of `selenium-webdriver`. This generator assumes the app was generated with the `--skip-test` flag, which means we need to add the `selenium-webdriver` and `capybara` gems. Updates `action_dispatch-testing-integration-capybara` dependency to the most recent tagged release in an effort to suppress Dependabot notifications. Ensure all files under `spec/support` are loaded by uncommenting a line generated by the RSpec installation script. [headless]: rails/rails#50512 [rspec]: rspec/rspec-rails#2746

JonRowe · 2024-09-07T09:39:49Z

Released in 7.0.0

stevepolitodesign force-pushed the update-default-driver branch from 15dff72 to 6f49c11 Compare March 16, 2024 00:08

Revert "[WIP] Attempt to fix CI failure"

a9b0295

This reverts commit f122744.

[WIP] Remove duplicate spec and add conditional to existing spec.

0036262

JonRowe added 2 commits March 19, 2024 21:01

Split spec per Rails version

633025b

Restore whitespace

937b2d9

JonRowe merged commit c2a7b82 into rspec:main Mar 19, 2024

JonRowe added a commit that referenced this pull request Mar 19, 2024

Follow up refactor for #2746 moving default driver to constant

5659af9

JonRowe added a commit that referenced this pull request Mar 19, 2024

Changelog for #2746

0c302c7

stevepolitodesign deleted the update-default-driver branch March 19, 2024 23:33

stevepolitodesign mentioned this pull request Mar 21, 2024

Testing Generator: Improvements thoughtbot/suspenders#1168

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use headless driver for next Rails release#2746

Use headless driver for next Rails release#2746
JonRowe merged 6 commits intorspec:mainfrom
stevepolitodesign:update-default-driver

stevepolitodesign commented Mar 16, 2024

Uh oh!

stevepolitodesign commented Mar 16, 2024

Uh oh!

JonRowe commented Mar 18, 2024

Uh oh!

stevepolitodesign commented Mar 18, 2024

Uh oh!

stevepolitodesign commented Mar 19, 2024

Uh oh!

JonRowe commented Mar 19, 2024

Uh oh!

stevepolitodesign commented Mar 19, 2024 •

edited

Loading

Uh oh!

JonRowe commented Mar 19, 2024

Uh oh!

stevepolitodesign commented Mar 19, 2024

Uh oh!

JonRowe commented Mar 19, 2024

Uh oh!

JonRowe commented Sep 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

stevepolitodesign commented Mar 16, 2024

Uh oh!

stevepolitodesign commented Mar 16, 2024

Uh oh!

JonRowe commented Mar 18, 2024

Uh oh!

stevepolitodesign commented Mar 18, 2024

Uh oh!

stevepolitodesign commented Mar 19, 2024

Uh oh!

JonRowe commented Mar 19, 2024

Uh oh!

stevepolitodesign commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonRowe commented Mar 19, 2024

Uh oh!

stevepolitodesign commented Mar 19, 2024

Uh oh!

JonRowe commented Mar 19, 2024

Uh oh!

JonRowe commented Sep 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

stevepolitodesign commented Mar 19, 2024 •

edited

Loading