From 59a30e8daef84e13966502420efd517597459d12 Mon Sep 17 00:00:00 2001 From: Myron Marston Date: Fri, 29 May 2015 01:03:11 -0700 Subject: [PATCH] Make backtrace truncation much more robust. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is necessary for the changes in rspec/rspec-support#210. That adds an additional stack frame to the parent exception (for the lambda that wraps `raise`) and our truncation wasn’t working properly with it. Really, the old way just happened to work. This is much more robust. --- .../core/formatters/exception_presenter.rb | 29 ++++- .../formatters/exception_presenter_spec.rb | 112 ++++++++++++++++++ 2 files changed, 138 insertions(+), 3 deletions(-) diff --git a/lib/rspec/core/formatters/exception_presenter.rb b/lib/rspec/core/formatters/exception_presenter.rb index 31bba7d55b..bd3bd0fceb 100644 --- a/lib/rspec/core/formatters/exception_presenter.rb +++ b/lib/rspec/core/formatters/exception_presenter.rb @@ -228,6 +228,8 @@ def multiple_failure_sumarizer(exception, prior_detail_formatter, color) end def sub_failure_list_formatter(exception, message_color) + common_backtrace_truncater = CommonBacktraceTruncater.new(exception) + lambda do |failure_number, colorizer, indentation| exception.all_exceptions.each_with_index.map do |failure, index| options = with_multiple_error_options_as_needed( @@ -238,9 +240,7 @@ def sub_failure_list_formatter(exception, message_color) :skip_shared_group_trace => true ) - failure = failure.dup - failure.set_backtrace(failure.backtrace[0..-exception.backtrace.size]) - + failure = common_backtrace_truncater.with_truncated_backtrace(failure) presenter = ExceptionPresenter.new(failure, @example, options) presenter.fully_formatted("#{failure_number}.#{index + 1}", colorizer) end.join @@ -255,6 +255,29 @@ def self.format_backtrace(*) [] end end + + # @private + class CommonBacktraceTruncater + def initialize(parent) + @parent = parent + end + + def with_truncated_backtrace(child) + child_bt = child.backtrace + parent_bt = @parent.backtrace + return child if child_bt.nil? || child_bt.empty? || parent_bt.nil? + + index_before_first_common_frame = -1.downto(-child_bt.size).find do |index| + parent_bt[index] != child_bt[index] + end + + return child if index_before_first_common_frame == -1 + + child = child.dup + child.set_backtrace(child_bt[0..index_before_first_common_frame]) + child + end + end end # @private diff --git a/spec/rspec/core/formatters/exception_presenter_spec.rb b/spec/rspec/core/formatters/exception_presenter_spec.rb index 174e6bb581..f4cbf51d26 100644 --- a/spec/rspec/core/formatters/exception_presenter_spec.rb +++ b/spec/rspec/core/formatters/exception_presenter_spec.rb @@ -194,4 +194,116 @@ def read_failed_line end end end + + RSpec.describe Formatters::ExceptionPresenter::Factory::CommonBacktraceTruncater do + def truncate(parent, child) + described_class.new(parent).with_truncated_backtrace(child) + end + + def exception_with(backtrace) + exception = Exception.new + exception.set_backtrace(backtrace) + exception + end + + it 'returns an exception with the common part truncated' do + parent = exception_with %w[ foo.rb:1 bar.rb:2 car.rb:7 ] + child = exception_with %w[ file_1.rb:3 file_1.rb:9 foo.rb:1 bar.rb:2 car.rb:7 ] + + truncated = truncate(parent, child) + + expect(truncated.backtrace).to eq %w[ file_1.rb:3 file_1.rb:9 ] + end + + it 'ignores excess lines in the top of the parent trace that the child does not have' do + parent = exception_with %w[ foo.rb:1 foo.rb:2 foo.rb:3 bar.rb:2 car.rb:7 ] + child = exception_with %w[ file_1.rb:3 file_1.rb:9 bar.rb:2 car.rb:7 ] + + truncated = truncate(parent, child) + + expect(truncated.backtrace).to eq %w[ file_1.rb:3 file_1.rb:9 ] + end + + it 'does not truncate anything if the parent has excess lines at the bottom of the trace' do + parent = exception_with %w[ foo.rb:1 bar.rb:2 car.rb:7 bazz.rb:9 ] + child = exception_with %w[ file_1.rb:3 file_1.rb:9 foo.rb:1 bar.rb:2 car.rb:7 ] + + truncated = truncate(parent, child) + + expect(truncated.backtrace).to eq %w[ file_1.rb:3 file_1.rb:9 foo.rb:1 bar.rb:2 car.rb:7 ] + end + + it 'does not mutate the provided exception' do + parent = exception_with %w[ foo.rb:1 bar.rb:2 car.rb:7 ] + child = exception_with %w[ file_1.rb:3 file_1.rb:9 foo.rb:1 bar.rb:2 car.rb:7 ] + + expect { truncate(parent, child) }.not_to change(child, :backtrace) + end + + it 'returns an exception with all the same attributes (except backtrace) as the provided one' do + parent = exception_with %w[ foo.rb:1 bar.rb:2 car.rb:7 ] + + my_custom_exception_class = Class.new(StandardError) do + attr_accessor :foo, :bar + end + + child = my_custom_exception_class.new("Some Message") + child.foo = 13 + child.bar = 20 + child.set_backtrace(%w[ foo.rb:1 ]) + + truncated = truncate(parent, child) + + expect(truncated).to have_attributes( + :message => "Some Message", + :foo => 13, + :bar => 20 + ) + end + + it 'handles child exceptions that have a blank array for the backtrace' do + parent = exception_with %w[ foo.rb:1 bar.rb:2 car.rb:7 ] + child = exception_with %w[ ] + + truncated = truncate(parent, child) + + expect(truncated.backtrace).to eq %w[ ] + end + + it 'handles child exceptions that have `nil` for the backtrace' do + parent = exception_with %w[ foo.rb:1 bar.rb:2 car.rb:7 ] + child = Exception.new + + truncated = truncate(parent, child) + + expect(truncated.backtrace).to be_nil + end + + it 'handles parent exceptions that have a blank array for the backtrace' do + parent = exception_with %w[ ] + child = exception_with %w[ foo.rb:1 ] + + truncated = truncate(parent, child) + + expect(truncated.backtrace).to eq %w[ foo.rb:1 ] + end + + it 'handles parent exceptions that have `nil` for the backtrace' do + parent = Exception.new + child = exception_with %w[ foo.rb:1 ] + + truncated = truncate(parent, child) + + expect(truncated.backtrace).to eq %w[ foo.rb:1 ] + end + + it 'returns the original exception object (not a dup) when there is no need to update the backtrace' do + parent = exception_with %w[ bar.rb:1 ] + child = exception_with %w[ foo.rb:1 ] + + truncated = truncate(parent, child) + + expect(truncated).to be child + end + end end