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

Preload Selenium driver_path before parallelizing system tests #49908

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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*
Expand Down
16 changes: 11 additions & 5 deletions actionpack/lib/action_dispatch/system_testing/browser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
26 changes: 20 additions & 6 deletions actionpack/test/dispatch/system_testing/driver_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down