From aca6c462a6af0eab53c7ea5820f2a9c65c5a88c7 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Wed, 4 Dec 2024 09:54:11 +1000
Subject: [PATCH] DEV: Improve rspec gem backtrace exclusion ENV vars (#30056)

Followup:

* https://github.com/discourse/discourse/pull/28160
* https://github.com/discourse/discourse/pull/25921

In the previous PRs we added 2 environent variables
to control backtrace output for errors in rspec,
`RSPEC_EXCLUDE_NOISE_IN_BACKTRACE`, and
`RSPEC_EXCLUDE_GEMS_IN_BACKTRACE`

These largely do the same thing, and we want to enable
that behaviour by default.

This commit consolidates them into one env var,
`DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE`, which is
disabled by default, meaning gem backtraces will not
be shown in rspec backtraces by default.

Also for the request spec use case with `RspecErrorTracker`,
we now show an indicator of how many lines were hidden from
the backtrace e.g. "...(21 framework line(s) excluded)",
and for this and the normal rspec backtrace exclusion we
show a warning if `DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE`
is not enabled.
---
 spec/rails_helper.rb | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index dd30745bbe2..7e78157d6bd 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -264,7 +264,10 @@ RSpec.configure do |config|
   # Sometimes the backtrace is quite big for failing specs, this will
   # remove rspec/gem paths from the backtrace so it's easier to see the
   # actual application code that caused the failure.
-  if ENV["RSPEC_EXCLUDE_NOISE_IN_BACKTRACE"]
+  #
+  # This behaviour is enabled by default, to include gems in
+  # the backtrace set DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE=1
+  if ENV["DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE"] != "1"
     config.backtrace_exclusion_patterns = [
       %r{/lib\d*/ruby/},
       %r{bin/},
@@ -686,23 +689,43 @@ RSpec.configure do |config|
   end
 
   config.after :each do |example|
+    # This behaviour is enabled by default, to include gems in
+    # the backtrace set DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE=1
+    if example.exception && ENV["DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE"] != "1"
+      lines = (RSpec.current_example.metadata[:extra_failure_lines] ||= +"")
+      lines << "Warning: DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE has not been enabled, gem backtrace will be excluded from the output\n"
+    end
+
     if example.exception && RspecErrorTracker.exceptions.present?
       lines = (RSpec.current_example.metadata[:extra_failure_lines] ||= +"")
 
+      lines << "\n"
       lines << "~~~~~~~ SERVER EXCEPTIONS ~~~~~~~"
+      lines << "\n"
 
       RspecErrorTracker.exceptions.each_with_index do |(path, ex), index|
         lines << "\n"
-        lines << "Error encountered while processing #{path}"
-        lines << "  #{ex.class}: #{ex.message}"
+        lines << "Error encountered while processing #{path}.\n"
+        lines << "  #{ex.class}: #{ex.message}\n"
+        framework_lines_excluded = 0
+
         ex.backtrace.each_with_index do |line, backtrace_index|
-          if ENV["RSPEC_EXCLUDE_GEMS_IN_BACKTRACE"]
-            next if line.match?(%r{/gems/})
+          if ENV["DISCOURSE_INCLUDE_GEMS_IN_RSPEC_BACKTRACE"] != "1"
+            if line.match?(%r{/gems/})
+              framework_lines_excluded += 1
+              next
+            else
+              if framework_lines_excluded.positive?
+                lines << "    ...(#{framework_lines_excluded} framework line(s) excluded)\n"
+                framework_lines_excluded = 0
+              end
+            end
           end
           lines << "    #{line}\n"
         end
       end
 
+      lines << "\n"
       lines << "~~~~~~~ END SERVER EXCEPTIONS ~~~~~~~"
       lines << "\n"
     end