forward system test driver options configured

in initializer block to the selenium driver
for non-headless browsers

* refactored browser options initialization.
* improved method names in AD::SystemTesting::Browser
* improved AD::SystemTest driver tests

actionpack/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Fix system test driver option initialization for non-headless browsers.
+
+    *glaszig*
+
 *   `redirect_to.action_controller` notifications now include the `ActionDispatch::Request` in
     their payloads as `:request`.
 

actionpack/lib/action_dispatch/system_testing/browser.rb

@@ -3,10 +3,11 @@
 module ActionDispatch
   module SystemTesting
     class Browser # :nodoc:
-      attr_reader :name
+      attr_reader :name, :options
 
       def initialize(name)
         @name = name
+        set_default_options
       end
 
       def type
@@ -20,23 +21,9 @@ def type
         end
       end
 
-      def options
+      def configure
-        case name
+        initialize_options
-        when :headless_chrome
+        yield options if block_given? && options
-          headless_chrome_browser_options
-        when :headless_firefox
-          headless_firefox_browser_options
-        end
-      end
-
-      def capabilities
-        @option ||=
-          case type
-          when :chrome
-            ::Selenium::WebDriver::Chrome::Options.new
-          when :firefox
-            ::Selenium::WebDriver::Firefox::Options.new
-          end
       end
 
       # driver_path can be configured as a proc. The webdrivers gem uses this
@@ -63,17 +50,37 @@ def preload
       end
 
       private
-        def headless_chrome_browser_options
+        def initialize_options
-          capabilities.args << "--headless"
+          @options ||= begin
-          capabilities.args << "--disable-gpu" if Gem.win_platform?
+            case type
-
+            when :chrome
-          capabilities
+              ::Selenium::WebDriver::Chrome::Options.new
+            when :firefox
+              ::Selenium::WebDriver::Firefox::Options.new
+            end
+          end
         end
 
-        def headless_firefox_browser_options
+        def set_default_options
-          capabilities.args << "-headless"
+          case name
+          when :headless_chrome
+            set_headless_chrome_browser_options
+          when :headless_firefox
+            set_headless_firefox_browser_options
+          end
+        end
 
-          capabilities
+        def set_headless_chrome_browser_options
+          configure do |capabilities|
+            capabilities.args << "--headless"
+            capabilities.args << "--disable-gpu" if Gem.win_platform?
+          end
+        end
+
+        def set_headless_firefox_browser_options
+          configure do |capabilities|
+            capabilities.args << "-headless"
+          end
         end
     end
   end

actionpack/lib/action_dispatch/system_testing/driver.rb

@@ -7,7 +7,7 @@ def initialize(name, **options, &capabilities)
         @name = name
         @browser = Browser.new(options[:using])
         @screen_size = options[:screen_size]
-        @options = options[:options]
+        @options = options[:options] || {}
         @capabilities = capabilities
 
         @browser.preload unless name == :rack_test
@@ -25,7 +25,7 @@ def registerable?
         end
 
         def register
-          define_browser_capabilities(@browser.capabilities)
+          @browser.configure(&@capabilities)
 
           Capybara.register_driver @name do |app|
             case @name
@@ -36,10 +36,6 @@ def register
           end
         end
 
-        def define_browser_capabilities(capabilities)
-          @capabilities.call(capabilities) if @capabilities
-        end
-
         def browser_options
           @options.merge(options: @browser.options).compact
         end

actionpack/test/dispatch/system_testing/driver_test.rb

@@ -56,61 +56,53 @@ class DriverTest < ActiveSupport::TestCase
   end
 
   test "define extra capabilities using chrome" do
-    driver_option = nil
     driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :chrome) do |option|
       option.add_argument("start-maximized")
       option.add_emulation(device_name: "iphone 6")
       option.add_preference(:detach, true)
-
-      driver_option = option
     end
     driver.use
+    browser_options = driver.__send__(:browser_options)
 
     expected = { "goog:chromeOptions" => { args: ["start-maximized"], mobileEmulation: { deviceName: "iphone 6" }, prefs: { detach: true } } }
-    assert_equal expected, driver_option.as_json
+    assert_equal expected, browser_options[:options].as_json
   end
 
   test "define extra capabilities using headless_chrome" do
-    driver_option = nil
     driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :headless_chrome) do |option|
       option.add_argument("start-maximized")
       option.add_emulation(device_name: "iphone 6")
       option.add_preference(:detach, true)
-
-      driver_option = option
     end
     driver.use
+    browser_options = driver.__send__(:browser_options)
 
-    expected = { "goog:chromeOptions" => { args: ["start-maximized"], mobileEmulation: { deviceName: "iphone 6" }, prefs: { detach: true } } }
+    expected = { "goog:chromeOptions" => { args: ["--headless", "start-maximized"], mobileEmulation: { deviceName: "iphone 6" }, prefs: { detach: true } } }
-    assert_equal expected, driver_option.as_json
+    assert_equal expected, browser_options[:options].as_json
   end
 
   test "define extra capabilities using firefox" do
-    driver_option = nil
     driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :firefox) do |option|
       option.add_preference("browser.startup.homepage", "http://www.seleniumhq.com/")
       option.add_argument("--host=127.0.0.1")
-
-      driver_option = option
     end
     driver.use
+    browser_options = driver.__send__(:browser_options)
 
     expected = { "moz:firefoxOptions" => { args: ["--host=127.0.0.1"], prefs: { "browser.startup.homepage" => "http://www.seleniumhq.com/" } } }
-    assert_equal expected, driver_option.as_json
+    assert_equal expected, browser_options[:options].as_json
   end
 
   test "define extra capabilities using headless_firefox" do
-    driver_option = nil
     driver = ActionDispatch::SystemTesting::Driver.new(:selenium, screen_size: [1400, 1400], using: :headless_firefox) do |option|
       option.add_preference("browser.startup.homepage", "http://www.seleniumhq.com/")
       option.add_argument("--host=127.0.0.1")
-
-      driver_option = option
     end
     driver.use
+    browser_options = driver.__send__(:browser_options)
 
-    expected = { "moz:firefoxOptions" => { args: ["--host=127.0.0.1"], prefs: { "browser.startup.homepage" => "http://www.seleniumhq.com/" } } }
+    expected = { "moz:firefoxOptions" => { args: ["-headless", "--host=127.0.0.1"], prefs: { "browser.startup.homepage" => "http://www.seleniumhq.com/" } } }
-    assert_equal expected, driver_option.as_json
+    assert_equal expected, browser_options[:options].as_json
   end
 
   test "does not define extra capabilities" do