Fix default host in setup, move teardown to helper file

* Override integration test default host

Integration tests automatically set the default host to
'http://example.com'. This works fine for integration tests because they
are not real browser sessions, but doesn't work fine for system tests
because they are real browser sessions.

We can override this by setting the `host!` in `before_setup. The
`Capybara.always_include_port` will allow the test to look at
`127.0.0.1:port capybara picks` and properly redirect the test.

Any application can override this by setting the `host!` in
their system test helper. Generally though, applications are going to be
using localhost.

In this commit I also moved the setup and teardown into their own module
for tidiness.

* Move teardown settings into system test case

These configuration options can be put into the system test case file
instead of the generated system tests helper file. This is an
implementation detail and therefore shouldn't be generated with the
template.

4 files changed, 39 insertions, 1 deletions

diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb
index 850f101ca8..91cbc1180c 100644
--- a/actionpack/lib/action_dispatch/system_test_case.rb
+++ b/actionpack/lib/action_dispatch/system_test_case.rb
@@ -4,6 +4,7 @@ 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"
 
 module ActionDispatch
   class SystemTestCase < IntegrationTest
@@ -92,6 +93,7 @@ module ActionDispatch
     # and Rails, any driver that is supported by Capybara is supported by system
     # tests as long as you include the required gems and files.
     include Capybara::DSL
+    include SystemTesting::TestHelpers::SetupAndTeardown
     include SystemTesting::TestHelpers::ScreenshotHelper
 
     def self.start_application # :nodoc:
diff --git a/actionpack/lib/action_dispatch/system_testing/server.rb b/actionpack/lib/action_dispatch/system_testing/server.rb
index 62ba07736b..4a214ef713 100644
--- a/actionpack/lib/action_dispatch/system_testing/server.rb
+++ b/actionpack/lib/action_dispatch/system_testing/server.rb
@@ -16,8 +16,17 @@ module ActionDispatch
         end
 
         def setup
+          set_server
+          set_port
+        end
+
+        def set_server
           Capybara.server = :rails_puma
         end
+
+        def set_port
+          Capybara.always_include_port = true
+        end
     end
   end
 end
diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb
new file mode 100644
index 0000000000..491559eedf
--- /dev/null
+++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb
@@ -0,0 +1,20 @@
+module ActionDispatch
+  module SystemTesting
+    module TestHelpers
+      module SetupAndTeardown # :nodoc:
+        DEFAULT_HOST = "127.0.0.1"
+
+        def before_setup
+          host! DEFAULT_HOST
+          super
+        end
+
+        def after_teardown
+          super
+          take_failed_screenshot
+          Capybara.reset_sessions!
+        end
+      end
+    end
+  end
+end
diff --git a/actionpack/test/dispatch/system_testing/server_test.rb b/actionpack/test/dispatch/system_testing/server_test.rb
index 66842f4ea9..10412d6367 100644
--- a/actionpack/test/dispatch/system_testing/server_test.rb
+++ b/actionpack/test/dispatch/system_testing/server_test.rb
@@ -3,8 +3,15 @@ require "capybara/dsl"
 require "action_dispatch/system_testing/server"
 
 class ServerTest < ActiveSupport::TestCase
+  setup do
+    ActionDispatch::SystemTesting::Server.new.run
+  end
+
   test "initializing the server port" do
-    server = ActionDispatch::SystemTesting::Server.new.run
     assert_includes Capybara.servers, :rails_puma
   end
+
+  test "port is always included" do
+    assert Capybara.always_include_port, "expected Capybara.always_include_port to be true"
+  end
 end