From be272a83a0165b5d6fb2a854cb58c43248e8b722 Mon Sep 17 00:00:00 2001
From: Jonathan Hefner <jonathan@hefner.pro>
Date: Thu, 7 Dec 2023 12:28:21 -0600
Subject: [PATCH] Fix system tests with Chrome cached by Selenium

Follow-up to #49908.

When Selenium resolves the driver path to a copy of Chrome that it has
downloaded / cached, it mutates the `Selenium::WebDriver::Chrome::Options`
object it receives, and relies on those changes later when the options
are used.  If `Selenium::WebDriver::Chrome::Service.driver_path` is set
but a different options object is used, Selenium will raise "cannot find
Chrome binary".  Therefore, this commit ensures that the options object
passed to `Selenium::WebDriver::DriverFinder.path` is the same options
object used by the driver later.
---
 .../action_dispatch/system_testing/browser.rb | 30 ++++++++-----------
 .../dispatch/system_testing/driver_test.rb    |  2 +-
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb
index e4d03ca5aa5a9..d86e66046fd2d 100644
--- a/actionpack/lib/action_dispatch/system_testing/browser.rb
+++ b/actionpack/lib/action_dispatch/system_testing/browser.rb
@@ -3,7 +3,7 @@
 module ActionDispatch
   module SystemTesting
     class Browser # :nodoc:
-      attr_reader :name, :options
+      attr_reader :name
 
       def initialize(name)
         @name = name
@@ -21,9 +21,18 @@ def type
         end
       end
 
+      def options
+        @options ||=
+          case type
+          when :chrome
+            ::Selenium::WebDriver::Chrome::Options.new
+          when :firefox
+            ::Selenium::WebDriver::Firefox::Options.new
+          end
+      end
+
       def configure
-        initialize_options
-        yield options if block_given? && options
+        yield options if block_given?
       end
 
       # driver_path is lazily initialized by default. Eagerly set it to
@@ -38,16 +47,6 @@ def preload
       end
 
       private
-        def initialize_options
-          @options ||=
-            case type
-            when :chrome
-              ::Selenium::WebDriver::Chrome::Options.new
-            when :firefox
-              ::Selenium::WebDriver::Firefox::Options.new
-            end
-        end
-
         def set_default_options
           case name
           when :headless_chrome
@@ -71,10 +70,7 @@ def set_headless_firefox_browser_options
         end
 
         def resolve_driver_path(namespace)
-          namespace::Service.driver_path = ::Selenium::WebDriver::DriverFinder.path(
-            options || namespace::Options.new,
-            namespace::Service
-          )
+          namespace::Service.driver_path = ::Selenium::WebDriver::DriverFinder.path(options, namespace::Service)
         end
     end
   end
diff --git a/actionpack/test/dispatch/system_testing/driver_test.rb b/actionpack/test/dispatch/system_testing/driver_test.rb
index c9d201fdf8247..7d443fd379c06 100644
--- a/actionpack/test/dispatch/system_testing/driver_test.rb
+++ b/actionpack/test/dispatch/system_testing/driver_test.rb
@@ -14,7 +14,7 @@ class DriverTest < ActiveSupport::TestCase
     driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :chrome, screen_size: [1400, 1400], options: { url: "http://example.com/wd/hub" })
     assert_equal :selenium, driver.instance_variable_get(:@driver_type)
     assert_equal :chrome, driver.instance_variable_get(:@browser).name
-    assert_nil driver.instance_variable_get(:@browser).options
+    assert_instance_of Selenium::WebDriver::Chrome::Options, driver.instance_variable_get(:@browser).options
     assert_equal [1400, 1400], driver.instance_variable_get(:@screen_size)
     assert_equal ({ url: "http://example.com/wd/hub" }), driver.instance_variable_get(:@options)
   end