Make screenshots default to "simple" format

Not everyone uses iTerm2 and whereas Terminal.app on a mac just ignores
that and outputs the path, other terminals like those on Ubuntu do not.
A friendlier default is one that works by default.

Closes #31159
Closes #30957

2 files changed, 13 insertions, 10 deletions

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 6c337cdc31..df0c5d3f0e 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
@@ -15,12 +15,11 @@ module ActionDispatch
         #
         # You can set the +RAILS_SYSTEM_TESTING_SCREENSHOT+ environment variable to
         # control the output. Possible values are:
-        # * [+inline+ (default)]    display the screenshot in the terminal using the
+        # * [+simple+ (default)]    Only displays the screenshot path.
+        #                           This is the default value.
+        # * [+inline+]              Display the screenshot in the terminal using the
         #                           iTerm image protocol (https://iterm2.com/documentation-images.html).
-        # * [+simple+]              only display the screenshot path.
-        #                           This is the default value if the +CI+ environment variables
-        #                           is defined.
-        # * [+artifact+]            display the screenshot in the terminal, using the terminal
+        # * [+artifact+]            Display the screenshot in the terminal, using the terminal
         #                           artifact format (https://buildkite.github.io/terminal/inline-images/).
         def take_screenshot
           save_image
@@ -59,11 +58,8 @@ module ActionDispatch
             # Environment variables have priority
             output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] || ENV["CAPYBARA_INLINE_SCREENSHOT"]
 
-            # If running in a CI environment, default to simple
-            output_type ||= "simple" if ENV["CI"]
-
-            # Default
-            output_type ||= "inline"
+            # Default to outputting a path to the screenshot
+            output_type ||= "simple"
 
             output_type
           end
diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb
index 2afda31cf5..264844fc7d 100644
--- a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb
+++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb
@@ -35,6 +35,11 @@ class ScreenshotHelperTest < ActiveSupport::TestCase
     end
   end
 
+  test "defaults to simple output for the screenshot" do
+    new_test = DrivenBySeleniumWithChrome.new("x")
+    assert_equal "simple", new_test.send(:output_type)
+  end
+
   test "display_image return artifact format when specify RAILS_SYSTEM_TESTING_SCREENSHOT environment" do
     begin
       original_output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"]
@@ -42,6 +47,8 @@ class ScreenshotHelperTest < ActiveSupport::TestCase
 
       new_test = DrivenBySeleniumWithChrome.new("x")
 
+      assert_equal "artifact", new_test.send(:output_type)
+
       Rails.stub :root, Pathname.getwd do
         new_test.stub :passed?, false do
           assert_match %r|url=artifact://.+?tmp/screenshots/failures_x\.png|, new_test.send(:display_image)