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

Allow system tests using Rack::Test to run without Chrome #37476

Merged
merged 2 commits into from Nov 20, 2019

Conversation

@sinsoku
Copy link
Contributor

sinsoku commented Oct 15, 2019

Summary

If you require "action_dispatch/system_test_case", the driven_by method will be executed immediately.

Then the preload method is called in SystemTesting::Driver#initialize.

Therefore, a "Webdrivers::BrowserNotFound" error occurs in the browser preloading when you run system tests witout Chrome.

This commit avoid the error by lazy configuring the driver.

ref: #37410

@rails-bot rails-bot bot added the actionpack label Oct 15, 2019
If you require "action_dispatch/system_test_case", the driven_by method
will be executed immediately.

* https://github.com/rails/rails/blob/v6.0.0/actionpack/lib/action_dispatch/system_test_case.rb#L162

Then the preload method is called in SystemTesting::Driver#initialize.

* https://github.com/rails/rails/blob/v6.0.0/actionpack/lib/action_dispatch/system_testing/driver.rb#L13
* https://github.com/rails/rails/blob/v6.0.0/actionpack/lib/action_dispatch/system_testing/browser.rb#L46-L63

Therefore, a "Webdrivers::BrowserNotFound" error occurs in the browser
preloading when you run system tests witout Chrome.

This commit avoid the error by lazy configuring the driver.

ref: #37410
@y-yagi

This comment has been minimized.

Copy link
Member

y-yagi commented Nov 1, 2019

In my understanding, is this need to fix Browser#preload call also?

Currently, Browser#preload calls regardless of drivers. If users specify like driven_by :rack_test, the type is chrome. So ::Selenium::WebDriver::Chrome::Service.driver_path will be call still.


if ::Selenium::WebDriver::Service.respond_to? :driver_path=
::Selenium::WebDriver::Chrome::Service.driver_path&.call

@sinsoku

This comment has been minimized.

Copy link
Contributor Author

sinsoku commented Nov 16, 2019

@y-yagi Thank you for your review.
I didn't notice about Browser#preload because the reproduction code specified driven_by(:rack_test, using: :rack_test). (refs: spec/system/foos_spec.rb)

I fixed it not to preload with :rack_test.

@@ -133,4 +133,17 @@ class DriverTest < ActiveSupport::TestCase
ensure
::Selenium::WebDriver::Chrome::Service.driver_path = original_driver_path
end

test "does not preload if :rack_test is set" do
called = false

This comment has been minimized.

Copy link
@y-yagi

y-yagi Nov 19, 2019

Member

We can use a helper method to check that method calls or not. Like the following:

  test "does not preload if :rack_test is set" do
    assert_not_called_on_instance_of(ActionDispatch::SystemTesting::Browser, :preload) do
      ActionDispatch::SystemTesting::Driver.new(:rack_test, using: :chrome)
    end
  end

This comment has been minimized.

Copy link
@sinsoku

sinsoku Nov 20, 2019

Author Contributor

I updated the code to use the assert_not_called_on_instance_of method and force-pushed it.

If users specify `driven_by(:rack_test)`, it uses Chrome by default arguments.
However, `Rack::Test` does not use a browser and does not need to be preloaded.

Furthermore, it occurs `Webdrivers::BrowserNotFound` when run in
a container (or a machine) without Chrome.

ref: #37410
@sinsoku sinsoku force-pushed the sinsoku:allow_rack_test_to_run_without_chrome branch from d0b1cc6 to e0e31b3 Nov 20, 2019
@y-yagi y-yagi merged commit ea303d0 into rails:master Nov 20, 2019
1 of 2 checks passed
1 of 2 checks passed
build
Details
buildkite/rails Build #65051 failed (39 minutes, 31 seconds)
Details
@y-yagi

This comment has been minimized.

Copy link
Member

y-yagi commented Nov 20, 2019

@sinsoku Thanks!

@sinsoku sinsoku deleted the sinsoku:allow_rack_test_to_run_without_chrome branch Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.