Refactor system test driver/browser

Since using a browser is only for selenium it doesn't really make sense
to have a separate class for handling it there. This brings a lot of the
if/else out of the main SystemTestCase class and into the Driver class
so we can abstract away all that extra work.

6 files changed, 35 insertions, 56 deletions

diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb
index d6388d8fb7..d872fca6bf 100644
--- a/actionpack/lib/action_dispatch/system_test_case.rb
+++ b/actionpack/lib/action_dispatch/system_test_case.rb
@@ -2,7 +2,6 @@ require "capybara/dsl"
 require "action_controller"
 require "action_dispatch/system_testing/driver"
 require "action_dispatch/system_testing/server"
-require "action_dispatch/system_testing/browser"
 require "action_dispatch/system_testing/test_helpers/screenshot_helper"
 require "action_dispatch/system_testing/test_helpers/setup_and_teardown"
 
@@ -105,21 +104,13 @@ module ActionDispatch
     #
     #   driven_by :selenium, screen_size: [800, 800]
     def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400])
-      driver = if selenium?(driver)
-                 SystemTesting::Browser.new(using, screen_size)
-               else
-                 SystemTesting::Driver.new(driver)
-               end
+      driver = SystemTesting::Driver.new(driver, using: using, screen_size: screen_size)
 
       setup { driver.use }
       teardown { driver.reset }
 
       SystemTesting::Server.new.run
     end
-
-    def self.selenium?(driver) # :nodoc:
-      driver == :selenium
-    end
   end
 
   SystemTestCase.start_application
diff --git a/actionpack/lib/action_dispatch/system_testing/browser.rb b/actionpack/lib/action_dispatch/system_testing/browser.rb
deleted file mode 100644
index 14ea06459d..0000000000
--- a/actionpack/lib/action_dispatch/system_testing/browser.rb
+++ /dev/null
@@ -1,27 +0,0 @@
-require "action_dispatch/system_testing/driver"
-
-module ActionDispatch
-  module SystemTesting
-    class Browser < Driver # :nodoc:
-      def initialize(name, screen_size)
-        super(name)
-        @name = name
-        @screen_size = screen_size
-      end
-
-      def use
-        register
-        super
-      end
-
-      private
-        def register
-          Capybara.register_driver @name do |app|
-            Capybara::Selenium::Driver.new(app, browser: @name).tap do |driver|
-              driver.browser.manage.window.size = Selenium::WebDriver::Dimension.new(*@screen_size)
-            end
-          end
-        end
-    end
-  end
-end
diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb
index 8decb54419..65e43d3584 100644
--- a/actionpack/lib/action_dispatch/system_testing/driver.rb
+++ b/actionpack/lib/action_dispatch/system_testing/driver.rb
@@ -1,18 +1,38 @@
 module ActionDispatch
   module SystemTesting
     class Driver # :nodoc:
-      def initialize(name)
+      def initialize(name, **options)
         @name = name
+        @browser = options[:using]
+        @screen_size = options[:screen_size]
       end
 
       def use
-        @current = Capybara.current_driver
-        Capybara.current_driver = @name
+        register if selenium?
+        setup
       end
 
       def reset
         Capybara.current_driver = @current
       end
+
+      private
+        def selenium?
+          @name == :selenium
+        end
+
+        def register
+          Capybara.register_driver @name do |app|
+            Capybara::Selenium::Driver.new(app, browser: @browser).tap do |driver|
+              driver.browser.manage.window.size = Selenium::WebDriver::Dimension.new(*@screen_size)
+            end
+          end
+        end
+
+        def setup
+          Capybara.current_driver = @name
+          @current = Capybara.current_driver
+        end
     end
   end
 end
diff --git a/actionpack/test/dispatch/system_testing/browser_test.rb b/actionpack/test/dispatch/system_testing/browser_test.rb
deleted file mode 100644
index b0ad309492..0000000000
--- a/actionpack/test/dispatch/system_testing/browser_test.rb
+++ /dev/null
@@ -1,10 +0,0 @@
-require "abstract_unit"
-require "action_dispatch/system_testing/browser"
-
-class BrowserTest < ActiveSupport::TestCase
-  test "initializing the browser" do
-    browser = ActionDispatch::SystemTesting::Browser.new(:chrome, [ 1400, 1400 ])
-    assert_equal :chrome, browser.instance_variable_get(:@name)
-    assert_equal [ 1400, 1400 ], browser.instance_variable_get(:@screen_size)
-  end
-end
diff --git a/actionpack/test/dispatch/system_testing/driver_test.rb b/actionpack/test/dispatch/system_testing/driver_test.rb
index f0ebdb38db..96d598a7a3 100644
--- a/actionpack/test/dispatch/system_testing/driver_test.rb
+++ b/actionpack/test/dispatch/system_testing/driver_test.rb
@@ -6,4 +6,15 @@ class DriverTest < ActiveSupport::TestCase
     driver = ActionDispatch::SystemTesting::Driver.new(:selenium)
     assert_equal :selenium, driver.instance_variable_get(:@name)
   end
+
+  test "initializing the driver with a browser" do
+    driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :chrome, screen_size: [ 1400, 1400 ])
+    assert_equal :selenium, driver.instance_variable_get(:@name)
+    assert_equal :chrome, driver.instance_variable_get(:@using)
+    assert_equal [ 1400, 1400 ], driver.instance_variable_get(:@screen_size)
+  end
+
+  test "selenium? returns false if driver is poltergeist" do
+    assert_not ActionDispatch::SystemTesting::Driver.new(:poltergeist).send(:selenium?)
+  end
 end
diff --git a/actionpack/test/dispatch/system_testing/system_test_case_test.rb b/actionpack/test/dispatch/system_testing/system_test_case_test.rb
index ff01d6739a..c9003e3841 100644
--- a/actionpack/test/dispatch/system_testing/system_test_case_test.rb
+++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb
@@ -1,11 +1,5 @@
 require "abstract_unit"
 
-class DrivenByCaseTestTest < ActiveSupport::TestCase
-  test "selenium? returns false if driver is poltergeist" do
-    assert_not ActionDispatch::SystemTestCase.selenium?(:poltergeist)
-  end
-end
-
 class DrivenByRackTestTest < ActionDispatch::SystemTestCase
   driven_by :rack_test