Merge pull request rails#49908 from mattbrictson/bugs/chromedriver-race-cond

Preload Selenium driver_path before parallelizing system tests

Author: jonathanhefner

actionpack/CHANGELOG.md

@@ -1,3 +1,8 @@
+*   Fix a race condition that could cause a `Text file busy - chromedriver`
+    error with parallel system tests
+
+    *Matt Brictson*
+
 *   Add `racc` as a dependency since it will become a bundled gem in Ruby 3.4.0
 
     *Hartley McGuire*

actionpack/lib/action_dispatch/system_testing/browser.rb

@@ -26,15 +26,14 @@ def configure
         yield options if block_given? && options
       end
 
-      # driver_path can be configured as a proc. Running this proc early allows
-      # us to only update the webdriver once and avoid race conditions when
-      # using parallel tests.
+      # driver_path is lazily initialized by default. Eagerly set it to
+      # avoid race conditions when using parallel tests.
       def preload
         case type
         when :chrome
-          ::Selenium::WebDriver::Chrome::Service.driver_path.try(:call)
+          resolve_driver_path(::Selenium::WebDriver::Chrome)
         when :firefox
-          ::Selenium::WebDriver::Firefox::Service.driver_path.try(:call)
+          resolve_driver_path(::Selenium::WebDriver::Firefox)
         end
       end
 
@@ -70,6 +69,13 @@ def set_headless_firefox_browser_options
             capabilities.add_argument("-headless")
           end
         end
+
+        def resolve_driver_path(namespace)
+          namespace::Service.driver_path = ::Selenium::WebDriver::DriverFinder.path(
+            options || namespace::Options.new,
+            namespace::Service
+          )
+        end
     end
   end
 end

actionpack/test/dispatch/system_testing/driver_test.rb

@@ -148,15 +148,29 @@ class DriverTest < ActiveSupport::TestCase
     end
   end
 
-  test "preloads browser's driver_path" do
-    called = false
-
+  test "preloads browser's driver_path with DriverFinder if a path isn't already specified" do
     original_driver_path = ::Selenium::WebDriver::Chrome::Service.driver_path
-    ::Selenium::WebDriver::Chrome::Service.driver_path = -> { called = true }
+    ::Selenium::WebDriver::Chrome::Service.driver_path = nil
 
-    ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :chrome)
+    # Our stub must return a path to a real executable, otherwise an internal Selenium assertion will fail.
+    found_executable = RbConfig.ruby
+    ::Selenium::WebDriver::SeleniumManager.stub(:driver_path, found_executable) do
+      ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :chrome)
+    end
 
-    assert called
+    assert_equal found_executable, ::Selenium::WebDriver::Chrome::Service.driver_path
+  ensure
+    ::Selenium::WebDriver::Chrome::Service.driver_path = original_driver_path
+  end
+
+  test "does not overwrite existing driver_path during preload" do
+    original_driver_path = ::Selenium::WebDriver::Chrome::Service.driver_path
+    # The driver_path must point to a real executable, otherwise an internal Selenium assertion will fail.
+    ::Selenium::WebDriver::Chrome::Service.driver_path = RbConfig.ruby
+
+    assert_no_changes -> { ::Selenium::WebDriver::Chrome::Service.driver_path } do
+      ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :chrome)
+    end
   ensure
     ::Selenium::WebDriver::Chrome::Service.driver_path = original_driver_path
   end