Call system test driver per-instance rather than globally

Previously the system test subclasses would call `driven_by` when the
app booted and not again when the test was initialized which resulted in
the driver from whichever class was called last to be used in tests.

In rails/rails#28144 the `driven_by` method was changed to run `use` on
setup and `reset` on teardown. While this was a viable fix this really
pointed to the problem that system test `driven_by` was a global
setting, rather than a per-class setting.

To alieviate this problem calling the driver should be done on an
instance level, rather than on the global level. I added an `initialize`
method to `SystemTestCase` which will call `use` on the superclass
driver. Running the server has been moved to `start_application` so that
it only needs to be called once on boot and no options from `driven_by`
were being passed to it.

This required a largish rewrite of the tests. Each test needs to utilize
the subclass so that it can properly test the drivers.
`ActionDispatch::SystemTestCase` shouldn't be called directly anymore.

7 files changed, 36 insertions, 42 deletions

diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb
index d872fca6bf..1bf47d2556 100644
--- a/actionpack/lib/action_dispatch/system_test_case.rb
+++ b/actionpack/lib/action_dispatch/system_test_case.rb
@@ -83,12 +83,19 @@ module ActionDispatch
     include SystemTesting::TestHelpers::SetupAndTeardown
     include SystemTesting::TestHelpers::ScreenshotHelper
 
+    def initialize(*) # :nodoc:
+      super
+      self.class.superclass.driver.use
+    end
+
     def self.start_application # :nodoc:
       Capybara.app = Rack::Builder.new do
         map "/" do
           run Rails.application
         end
       end
+
+      SystemTesting::Server.new.run
     end
 
     # System Test configuration options
@@ -104,12 +111,12 @@ module ActionDispatch
     #
     #   driven_by :selenium, screen_size: [800, 800]
     def self.driven_by(driver, using: :chrome, screen_size: [1400, 1400])
-      driver = SystemTesting::Driver.new(driver, using: using, screen_size: screen_size)
-
-      setup { driver.use }
-      teardown { driver.reset }
+      @driver = SystemTesting::Driver.new(driver, using: using, screen_size: screen_size)
+    end
 
-      SystemTesting::Server.new.run
+    # Returns the driver object for the initialized system test
+    def self.driver
+      @driver ||= SystemTestCase.driven_by(:selenium)
     end
   end
 
diff --git a/actionpack/lib/action_dispatch/system_testing/driver.rb b/actionpack/lib/action_dispatch/system_testing/driver.rb
index 65e43d3584..72d132d64f 100644
--- a/actionpack/lib/action_dispatch/system_testing/driver.rb
+++ b/actionpack/lib/action_dispatch/system_testing/driver.rb
@@ -12,10 +12,6 @@ module ActionDispatch
         setup
       end
 
-      def reset
-        Capybara.current_driver = @current
-      end
-
       private
         def selenium?
           @name == :selenium
@@ -31,7 +27,6 @@ module ActionDispatch
 
         def setup
           Capybara.current_driver = @name
-          @current = Capybara.current_driver
         end
     end
   end
diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb
index e37f6d02aa..6de8fb74dc 100644
--- a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb
+++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb
@@ -57,7 +57,7 @@ module ActionDispatch
           end
 
           def supports_screenshot?
-            page.driver.public_methods(false).include?(:save_screenshot)
+            Capybara.current_driver != :rack_test
           end
       end
     end
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index 459b0d6c54..4185ce1a1f 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -439,3 +439,11 @@ class ActiveSupport::TestCase
     skip message if defined?(JRUBY_VERSION)
   end
 end
+
+class DrivenByRackTest < ActionDispatch::SystemTestCase
+  driven_by :rack_test
+end
+
+class DrivenBySeleniumWithChrome < ActionDispatch::SystemTestCase
+  driven_by :selenium, using: :chrome
+end
diff --git a/actionpack/test/dispatch/system_testing/driver_test.rb b/actionpack/test/dispatch/system_testing/driver_test.rb
index 96d598a7a3..8f8777b19f 100644
--- a/actionpack/test/dispatch/system_testing/driver_test.rb
+++ b/actionpack/test/dispatch/system_testing/driver_test.rb
@@ -8,10 +8,10 @@ class DriverTest < ActiveSupport::TestCase
   end
 
   test "initializing the driver with a browser" do
-    driver = ActionDispatch::SystemTesting::Driver.new(:selenium, using: :chrome, screen_size: [ 1400, 1400 ])
+    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)
+    assert_equal :chrome, driver.instance_variable_get(:@browser)
+    assert_equal [1400, 1400], driver.instance_variable_get(:@screen_size)
   end
 
   test "selenium? returns false if driver is poltergeist" do
diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb
index d6b501b3ac..a83818fd80 100644
--- a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb
+++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb
@@ -4,13 +4,13 @@ require "capybara/dsl"
 
 class ScreenshotHelperTest < ActiveSupport::TestCase
   test "image path is saved in tmp directory" do
-    new_test = ActionDispatch::SystemTestCase.new("x")
+    new_test = DrivenBySeleniumWithChrome.new("x")
 
     assert_equal "tmp/screenshots/x.png", new_test.send(:image_path)
   end
 
   test "image path includes failures text if test did not pass" do
-    new_test = ActionDispatch::SystemTestCase.new("x")
+    new_test = DrivenBySeleniumWithChrome.new("x")
 
     new_test.stub :passed?, false do
       assert_equal "tmp/screenshots/failures_x.png", new_test.send(:image_path)
@@ -18,7 +18,7 @@ class ScreenshotHelperTest < ActiveSupport::TestCase
   end
 
   test "image path does not include failures text if test skipped" do
-    new_test = ActionDispatch::SystemTestCase.new("x")
+    new_test = DrivenBySeleniumWithChrome.new("x")
 
     new_test.stub :passed?, false do
       new_test.stub :skipped?, true do
@@ -26,28 +26,16 @@ class ScreenshotHelperTest < ActiveSupport::TestCase
       end
     end
   end
+end
 
+class RackTestScreenshotsTest < DrivenByRackTest
   test "rack_test driver does not support screenshot" do
-    begin
-      original_driver = Capybara.current_driver
-      Capybara.current_driver = :rack_test
-
-      new_test = ActionDispatch::SystemTestCase.new("x")
-      assert_not new_test.send(:supports_screenshot?)
-    ensure
-      Capybara.current_driver = original_driver
-    end
+    assert_not self.send(:supports_screenshot?)
   end
+end
 
+class SeleniumScreenshotsTest < DrivenBySeleniumWithChrome
   test "selenium driver supports screenshot" do
-    begin
-      original_driver = Capybara.current_driver
-      Capybara.current_driver = :selenium
-
-      new_test = ActionDispatch::SystemTestCase.new("x")
-      assert new_test.send(:supports_screenshot?)
-    ensure
-      Capybara.current_driver = original_driver
-    end
+    assert self.send(:supports_screenshot?)
   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 c9003e3841..1a9421c098 100644
--- a/actionpack/test/dispatch/system_testing/system_test_case_test.rb
+++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb
@@ -1,17 +1,13 @@
 require "abstract_unit"
 
-class DrivenByRackTestTest < ActionDispatch::SystemTestCase
-  driven_by :rack_test
-
+class SetDriverToRackTestTest < DrivenByRackTest
   test "uses rack_test" do
     assert_equal :rack_test, Capybara.current_driver
   end
 end
 
-class DrivenBySeleniumWithChromeTest < ActionDispatch::SystemTestCase
-  driven_by :selenium, using: :chrome
-
+class SetDriverToSeleniumTest < DrivenBySeleniumWithChrome
   test "uses selenium" do
-    assert_equal :chrome, Capybara.current_driver
+    assert_equal :selenium, Capybara.current_driver
   end
 end