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
rails · Nov 29, 2017 · 9d6e288 · 9d6e288
1 parent 527ad1a
commit 9d6e288
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 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
@@ -15,12 +15,11 @@ module ScreenshotHelper
         #
         # 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 @@ def output_type
             # 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
@@ -35,13 +35,20 @@ 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"]
       ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = "artifact"
 
       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)