Skip to content

Commit

Permalink
Make backtrace truncation much more robust.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
myronmarston committed Jun 1, 2015
1 parent 4fa21d0 commit 59a30e8
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 3 deletions.
29 changes: 26 additions & 3 deletions lib/rspec/core/formatters/exception_presenter.rb
Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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
Expand Down
112 changes: 112 additions & 0 deletions spec/rspec/core/formatters/exception_presenter_spec.rb
Expand Up @@ -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

0 comments on commit 59a30e8

Please sign in to comment.