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

Rails 7.1.2 system tests won't find Chrome downloaded by selenium-webdriver #50287

Closed
lazyatom opened this issue Dec 6, 2023 · 3 comments · Fixed by #50296
Closed

Rails 7.1.2 system tests won't find Chrome downloaded by selenium-webdriver #50287

lazyatom opened this issue Dec 6, 2023 · 3 comments · Fixed by #50296
Assignees

Comments

@lazyatom
Copy link
Contributor

lazyatom commented Dec 6, 2023

With a recent change (#49908), if you don't have the Chrome (or Chromium) browser accessible in your default system path, system tests will fail to run.

It's not necessary to have Chrome installed to use Selenium with Chrome, because the chromedriver binary will automatically download a version and store it in a cache directory for you.

I believe this error is caused by the assignment to namespace::Service.driver_path:

namespace::Service.driver_path = ::Selenium::WebDriver::DriverFinder.path( ... )

Internally within selenium-webdriver, if this value is assigned, some important behaviour is skipped when this code runs within selenium-webdriver:

        path = klass.driver_path
        path = path.call if path.is_a?(Proc)

        path ||= begin
          SeleniumManager.driver_path(options) unless options.is_a?(Remote::Capabilities)
        rescue StandardError => e
          raise Error::NoSuchDriverError, "Unable to obtain #{klass::EXECUTABLE} using Selenium Manager; #{e.message}"
        end

One side-effect of the SeleniumManager.driver_path(options) method call is that the location of any Chrome binary is inserted into the options Hash, which is then used in other parts of the system and ultimately sent as part of the "capabilities" to the chromedriver process.

However, if klass.driver_path is already set -- which is what the change in #49908 does -- then the begin .. end block is not evaluated, and so the location of any Chrome binary that selenium-webdrivers finds is never inserted into the options Hash.

In my script below I've enabled debug logging from selenium-webdriver, and within the logging, you'll see this:

ChromeDriver was started successfully.
2023-12-06 20:25:49 DEBUG Selenium [:command] -> POST session 
2023-12-06 20:25:49 DEBUG Selenium [:command]    >>> http://127.0.0.1:9519/session | {"capabilities":{"alwaysMatch":{"browserName":"chrome","goog:chromeOptions":{}}}} 

If I disable the change made in #49908 (e.g. by uncommenting the part of the script below that changes the preload method into a no-op), you'll see this in the logging instead:

ChromeDriver was started successfully.
2023-12-06 20:28:55 DEBUG Selenium [:command] -> POST session 
2023-12-06 20:28:55 DEBUG Selenium [:command]    >>> http://127.0.0.1:9519/session | {"capabilities":{"alwaysMatch":{"browserName":"chrome","goog:chromeOptions":{"binary":"/Users/james/.cache/selenium/chrome/mac-arm64/120.0.6099.62/Google Chrome for Testing.app/Contents/MacOS/Google Chrome for Testing"}}}} 

Crucially, the "binary" value is present, which tells chromedriver how to actually launch the specific instance of Chrome that the gem has downloaded on our behalf.

Steps to reproduce

Most importantly, you must not have Chrome or Chromium installed via any other mechanism.

This script is sufficient to replicate the issue for me:

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails", branch: "main"
  gem "capybara"
  gem "selenium-webdriver"
end

require "action_controller/railtie"
require "minitest/autorun"

class TestApp < Rails::Application
  config.root = __dir__
  config.hosts << "example.org"
  config.secret_key_base = "secret_key_base"

  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger

  routes.draw do
    get "/" => "test#index"
  end
end

class TestController < ActionController::Base
  include Rails.application.routes.url_helpers

  def index
    render plain: "Home"
  end
end

Selenium::WebDriver.logger.level = :debug

# Uncomment this to see the expected behaviour
# 
# require 'action_dispatch/system_testing/browser'
# module ActionDispatch
#   module SystemTesting
#     class Browser
#       def preload
#         puts "Skipping new preload behaviour entirely" 
#         return
#       end
#     end
#   end
# end

class BugTest < ActionDispatch::SystemTestCase
  driven_by :selenium, using: :chrome, screen_size: [1400, 1400]

  def test_stuff
    visit "/"
  end
end

Expected behavior

The test should run without any errors.

Actual behavior

An exception is raised:

Selenium::WebDriver::Error::UnknownError: unknown error: cannot find Chrome binary
    0   chromedriver                        0x000000010451c4f4 chromedriver + 4162804
    1   chromedriver                        0x000000010451467c chromedriver + 4130428
    2   chromedriver                        0x000000010416bbc0 chromedriver + 293824
    3   chromedriver                        0x000000010419b704 chromedriver + 489220
    4   chromedriver                        0x000000010419a128 chromedriver + 483624
    5   chromedriver                        0x00000001041e0c40 chromedriver + 773184
    6   chromedriver                        0x00000001041a56bc chromedriver + 530108
    7   chromedriver                        0x00000001041a6930 chromedriver + 534832
    8   chromedriver                        0x00000001044e1e20 chromedriver + 3923488
    9   chromedriver                        0x00000001044e63f4 chromedriver + 3941364
    10  chromedriver                        0x00000001044ca050 chromedriver + 3825744
    11  chromedriver                        0x00000001044e6f54 chromedriver + 3944276
    12  chromedriver                        0x00000001044bc70c chromedriver + 3770124
    13  chromedriver                        0x0000000104503998 chromedriver + 4061592
    14  chromedriver                        0x0000000104503b10 chromedriver + 4061968
    15  chromedriver                        0x00000001045142fc chromedriver + 4129532
    16  libsystem_pthread.dylib             0x000000018d431034 _pthread_start + 136
    17  libsystem_pthread.dylib             0x000000018d42be3c thread_start + 8
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/remote/response.rb:55:in `assert_ok'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/remote/response.rb:34:in `initialize'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/remote/http/common.rb:83:in `new'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/remote/http/common.rb:83:in `create_response'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/remote/http/default.rb:103:in `request'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/remote/http/common.rb:59:in `call'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/remote/bridge.rb:601:in `execute'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/remote/bridge.rb:53:in `create_session'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/common/driver.rb:317:in `block in create_bridge'
    <internal:kernel>:90:in `tap'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/common/driver.rb:316:in `create_bridge'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/common/driver.rb:74:in `initialize'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/chrome/driver.rb:35:in `initialize'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/common/driver.rb:47:in `new'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver/common/driver.rb:47:in `for'
    /Users/james/.gem/ruby/3.2.2/gems/selenium-webdriver-4.16.0/lib/selenium/webdriver.rb:89:in `for'
    /Users/james/.gem/ruby/3.2.2/gems/capybara-3.39.2/lib/capybara/selenium/driver.rb:83:in `browser'
    /Users/james/.gem/ruby/3.2.2/bundler/gems/rails-c057edaaadbe/actionpack/lib/action_dispatch/system_testing/driver.rb:55:in `block in register_selenium'
    <internal:kernel>:90:in `tap'
    /Users/james/.gem/ruby/3.2.2/bundler/gems/rails-c057edaaadbe/actionpack/lib/action_dispatch/system_testing/driver.rb:54:in `register_selenium'
    /Users/james/.gem/ruby/3.2.2/bundler/gems/rails-c057edaaadbe/actionpack/lib/action_dispatch/system_testing/driver.rb:41:in `block in register'
    /Users/james/.gem/ruby/3.2.2/gems/capybara-3.39.2/lib/capybara/session.rb:105:in `driver'
    /Users/james/.gem/ruby/3.2.2/gems/capybara-3.39.2/lib/capybara/session.rb:91:in `initialize'
    /Users/james/.gem/ruby/3.2.2/gems/capybara-3.39.2/lib/capybara.rb:421:in `new'
    /Users/james/.gem/ruby/3.2.2/gems/capybara-3.39.2/lib/capybara.rb:421:in `block in session_pool'
    /Users/james/.gem/ruby/3.2.2/gems/capybara-3.39.2/lib/capybara.rb:317:in `current_session'
    /Users/james/.gem/ruby/3.2.2/gems/capybara-3.39.2/lib/capybara/dsl.rb:46:in `page'
    /Users/james/.gem/ruby/3.2.2/gems/capybara-3.39.2/lib/capybara/dsl.rb:52:in `visit'
    selenium-webdriver-issue.rb:43:in `test_stuff'

System configuration

Rails version: 7.1.2, edge

Ruby version: 3.2.2

Chrome version: importantly, not installed!

@mattbrictson
Copy link
Contributor

Could this be the problem? When preloading, we use the existing options if the browser was already configured, but otherwise we create a "throwaway" options object.

namespace::Service.driver_path = ::Selenium::WebDriver::DriverFinder.path(
  options || namespace::Options.new, # <-- throwaway object
  namespace::Service
)

Therefore, if DriverFinder.path mutates those options (e.g. assigns goog:chromeOptions.binary), those mutations are lost when using the throwaway object.

If this is indeed the problem, then a solution would be to force options to be initialized with initialize_options prior to calling DriverFinder.path, like this:

def resolve_driver_path(namespace)
  initialize_options
  namespace::Service.driver_path = ::Selenium::WebDriver::DriverFinder.path(
    options, # <-- mutations are now retained
    namespace::Service
  )
end

That said, I don't really understand why options is being lazily initialized in the first place. Eagerly initializing it might have unforeseen consequences.

@lazyatom
Copy link
Contributor Author

lazyatom commented Dec 7, 2023

a solution would be to force options to be initialized with initialize_options prior to calling DriverFinder.path

Yeah, that seems to work. When I was exploring this, I wasn't sure if the options in ActionDispatch::SystemTesting::Browser was the same object that ultimately gets passed around inside selenium-webdrivers, but I've traced the object ID and it certainly seems to be.

I've tried to figure out if there's any clear reason to lazy-initialize the options value. If I move the call to initialize_options into the AD::SystemTesting::Browser#initialize method, there is a test failure -- https://github.com/rails/rails/blob/main/actionpack/test/dispatch/system_testing/driver_test.rb#L17 -- but there's nothing in the commit that introduced that test, or test itself, that explains why that value should or must be nil (as opposed to asserting what options are actually set). My hunch is that it doesn't matter.

@jonathanhefner
Copy link
Member

Could this be the problem? When preloading, we use the existing options if the browser was already configured, but otherwise we create a "throwaway" options object.

That was my conclusion as well.

but there's nothing in the commit that introduced that test, or test itself, that explains why that value should or must be nil (as opposed to asserting what options are actually set). My hunch is that it doesn't matter.

I believe you are correct. There doesn't seem to be a reason for that assertion. I think it is asserting incidental behavior from f52e17f, not expected behavior.

Likewise, there doesn't seem to be a clear reason for a separate initialize_options method (introduced by 769188e), instead of lazily computing @options in an options method.

So I have submitted #50296 to address this issue and clean up the code a bit.

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 a pull request may close this issue.

3 participants