diff --git a/rspec-core/Changelog.md b/rspec-core/Changelog.md index 1598f0e4a..78970eb5d 100644 --- a/rspec-core/Changelog.md +++ b/rspec-core/Changelog.md @@ -5,6 +5,9 @@ Bug Fixes: * Emit a warning when `around` hook is used with `:context` scope (Phil Pirozhkov, #2687) +* Prevent invalid implementations of `Exception#cause` from being treated as a + valid cause (and causing strange errors) in `RSpec::Core::Formatters::ExceptionPresenter`. + (Jon Rowe, #2703) ### 3.9.1 / 2019-12-28 [Full Changelog](http://github.com/rspec/rspec-core/compare/v3.9.0...v3.9.1) diff --git a/rspec-core/lib/rspec/core/formatters/exception_presenter.rb b/rspec-core/lib/rspec/core/formatters/exception_presenter.rb index 9ad6503e1..db612e885 100644 --- a/rspec-core/lib/rspec/core/formatters/exception_presenter.rb +++ b/rspec-core/lib/rspec/core/formatters/exception_presenter.rb @@ -43,7 +43,7 @@ def formatted_backtrace(exception=@exception) if RSpec::Support::RubyFeatures.supports_exception_cause? def formatted_cause(exception) - last_cause = final_exception(exception) + last_cause = final_exception(exception, [exception]) cause = [] if exception.cause @@ -55,7 +55,9 @@ def formatted_cause(exception) cause << " #{line}" end - cause << (" #{backtrace_formatter.format_backtrace(last_cause.backtrace, example.metadata).first}") + unless last_cause.backtrace.empty? + cause << (" #{backtrace_formatter.format_backtrace(last_cause.backtrace, example.metadata).first}") + end end cause @@ -96,7 +98,8 @@ def fully_formatted_lines(failure_number, colorizer) def final_exception(exception, previous=[]) cause = exception.cause - if cause && !previous.include?(cause) + + if cause && Exception === cause && !previous.include?(cause) previous << cause final_exception(cause, previous) else diff --git a/rspec-core/spec/rspec/core/formatters/exception_presenter_spec.rb b/rspec-core/spec/rspec/core/formatters/exception_presenter_spec.rb index 347524cc2..104673fa5 100644 --- a/rspec-core/spec/rspec/core/formatters/exception_presenter_spec.rb +++ b/rspec-core/spec/rspec/core/formatters/exception_presenter_spec.rb @@ -11,7 +11,22 @@ module RSpec::Core before do allow(example.execution_result).to receive(:exception) { exception } example.metadata[:absolute_file_path] = __FILE__ - allow(exception).to receive(:cause) if RSpec::Support::RubyFeatures.supports_exception_cause? + end + + # This is a slightly more realistic exception than our instance_double + # created, as this will behave correctly with `Exception#===`, note we + # monkey patch the backtrace / cause in because these are not public + # api but we need specific values for our fakes. + class FakeException < Exception + def initialize(message, backtrace = [], cause = nil) + super(message) + @backtrace = backtrace + @cause = cause + end + attr_reader :backtrace + if RSpec::Support::RubyFeatures.supports_exception_cause? + attr_accessor :cause + end end describe "#fully_formatted" do @@ -25,7 +40,7 @@ module RSpec::Core line_num = __LINE__ + 1 # The failure happened here! Handles encoding too! ЙЦ end - let(:exception) { instance_double(Exception, :message => "Boom\nBam", :backtrace => [ "#{__FILE__}:#{line_num}"]) } + let(:exception) { FakeException.new("Boom\nBam", [ "#{__FILE__}:#{line_num}"]) } it "formats the exception with all the normal details" do expect(presenter.fully_formatted(1)).to eq(<<-EOS.gsub(/^ +\|/, '')) @@ -159,16 +174,14 @@ module RSpec::Core EOS end - let(:the_exception) { instance_double(Exception, :cause => second_exception, :message => "Boom\nBam", :backtrace => [ "#{__FILE__}:#{line_num}"]) } + let(:the_exception) { FakeException.new("Boom\nBam", [ "#{__FILE__}:#{line_num}"], second_exception) } let(:second_exception) do - instance_double(Exception, :cause => first_exception, :message => "Second\nexception", :backtrace => ["#{__FILE__}:#{__LINE__}"]) + FakeException.new("Second\nexception", ["#{__FILE__}:#{__LINE__}"], first_exception) end - caused_by_line_num = __LINE__ + 2 - let(:first_exception) do - instance_double(Exception, :cause => nil, :message => "Real\nculprit", :backtrace => ["#{__FILE__}:#{__LINE__}"]) - end + caused_by_line_num = __LINE__ + 1 + let(:first_exception) { FakeException.new("Real\nculprit", ["#{__FILE__}:#{__LINE__}"]) } it 'includes the first exception that caused the failure', :if => RSpec::Support::RubyFeatures.supports_exception_cause? do the_presenter = Formatters::ExceptionPresenter.new(the_exception, example) @@ -211,8 +224,9 @@ module RSpec::Core it 'wont produce a stack error when the cause is an older exception', :if => RSpec::Support::RubyFeatures.supports_exception_cause? do allow(the_exception).to receive(:cause) do - instance_double(Exception, :cause => the_exception, :message => "A loop", :backtrace => the_exception.backtrace) + FakeException.new("A loop", the_exception.backtrace, the_exception) end + the_presenter = Formatters::ExceptionPresenter.new(the_exception, example) expect(the_presenter.fully_formatted(1)).to eq(<<-EOS.gsub(/^ +\|/, '')) @@ -230,6 +244,22 @@ module RSpec::Core EOS end + it 'will work when cause is incorrectly overridden', :if => RSpec::Support::RubyFeatures.supports_exception_cause? do + incorrect_cause_exception = FakeException.new("A badly implemented exception", [], "An incorrect cause") + + the_presenter = Formatters::ExceptionPresenter.new(incorrect_cause_exception, example) + + expect(the_presenter.fully_formatted(1)).to eq(<<-EOS.gsub(/^ +\|/, '')) + | + | 1) Example + | Failure/Error: Unable to find matching line from backtrace + | A badly implemented exception + | # ------------------ + | # --- Caused by: --- + | # A badly implemented exception + EOS + end + it "adds extra failure lines from the example metadata" do extra_example = example.clone failure_line = 'http://www.example.com/job_details/123'