Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Include backtrace in errors reported by `raise_error` matcher. #177

Merged
merged 1 commit into from

3 participants

@myronmarston
Owner

It's hard to troubleshoot unexpected errors when the backtrace is silenced,
as it was previously.

Closes #59.

@myronmarston
Owner

Note this depends on rspec/rspec-core#701.

@alindeman / @dchelimsky -- can you review?

lib/rspec/matchers/configuration.rb
((6 lines not shown))
+ # implement `#format_backtrace(Array<String>)`. This is used
+ # to format backtraces of errors handled by the `raise_error`
+ # matcher.
+ #
+ # If you are using rspec-core, rspec-core's backtrace formatting
+ # will be used (including respecting the presence or absence of
+ # the `--backtrace` option).
+ #
+ # @overload backtrace_formatter
+ # @return [#format_backtrace] the backtrace formatter
+ # @overload backtrace_formatter=
+ # @param value [#format_backtrace] sets the backtrace formatter
+ attr_writer :backtrace_formatter
+ def backtrace_formatter
+ @backtrace_formatter ||= if defined?(::RSpec::Core::Formatters::Helpers)
+ ::RSpec::Core::Formatters::Helpers
@dchelimsky Owner

I'd rather expose this more explicitly in RSpec::Core - something like RSpec::Core::BacktraceFormatters. WDYT?

@myronmarston Owner

Agreed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
spec/rspec/matchers/raise_error_spec.rb
@@ -51,6 +51,27 @@
lambda { raise RuntimeError, "example message" }.should_not raise_error
}.should fail_with(/expected no Exception, got #<RuntimeError: example message>/)
end
+
+ it 'includes the backtrace of the error that was raised in the error message' do
+ expect {
+ expect { raise "boom" }.not_to raise_error
+ }.to raise_error { |e|
+ backtrace_line = "#{File.basename(__FILE__)}:#{__LINE__ - 2}"
+ e.message.should include("with backtrace", backtrace_line)
+ }
+ end
+
+ it 'formats the backtrace using the configured backtrace formatter' do
+ RSpec::Matchers.configuration.backtrace_formatter.
+ should_receive(:format_backtrace).
@dchelimsky Owner

I prefer to reserve message expectations for cases where any side effects or state changes happen upstream. In this example, a stub would tell the story just as well since the literal "formatted-backtrace" appears twice in the example. Subtle, and subjective, I realize. Don't change it if you feel strongly about it.

@myronmarston Owner

I'm on the fence about this...mocks are best used with commands, not queries (and this is a query), but given the focus of the example is on "using the configured backtrace formatter", should_receive seemed appropriate here. The return value still needed to be used because format_backtrace could be called and then the matcher could throw the return value away and not use it.

But you're right, the use of the return value (if it was just a stub) demonstrates it is used just fine.

@alindeman -- do you have a preference?

@alindeman Collaborator

I'd probably personally use a stub here, for the same reasons as you and @dchelimsky mention. That said, I don't feel strongly about it in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
spec/rspec/matchers/configuration_spec.rb
((7 lines not shown))
+
+ let(:formatted_backtrace) do
+ config.backtrace_formatter.format_backtrace(original_backtrace)
+ end
+
+ before do
+ RSpec.configuration.stub(:backtrace_clean_patterns) { [/clean-me/] }
+ end
+
+ it "defaults to rspec-core's backtrace formatter when rspec-core is loaded" do
+ expect(config.backtrace_formatter).to be(RSpec::Core::Formatters::Helpers)
+ expect(formatted_backtrace).to eq(cleaned_backtrace)
+ end
+
+ it "defaults to a null formatter when rspec-core is not loaded" do
+ stub_const("RSpec::Core::Formatters", nil) # so the formatter module is not loaded
@dchelimsky Owner

Nice example of stub_const. It's a big hammer, but this is a perfect case for it!

@myronmarston Owner

Actually, this would be a perfect case for rspec/rspec-mocks#183, but we haven't implemented that yet, and this works OK here :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@myronmarston
Owner

One other thing to be fixed here:

expect {
  blah
}.to raise_error { |e|
  expect(e.some_attribute).to eq(some_value)
}

In a case like this, if the expectation in the error-matching block fails, the backtrace is not being included in the failure. It'd be cool to find a solution for this (thought it might be complex, in which case I might skip it).

@myronmarston myronmarston Include backtrace in errors reported by `raise_error` matcher.
It's hard to troubleshoot unexpected errors when the backtrace is silenced,
as it was previously.

Closes #59.
4a919ab
@myronmarston myronmarston merged commit 16a133f into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 9, 2012
  1. @myronmarston

    Include backtrace in errors reported by `raise_error` matcher.

    myronmarston authored
    It's hard to troubleshoot unexpected errors when the backtrace is silenced,
    as it was previously.
    
    Closes #59.
This page is out of date. Refresh to see the latest.
View
2  Changelog.md
@@ -4,6 +4,8 @@
Enhancements
* Colorize diffs if the `--color` option is configured. (Alex Coplan)
+* Include backtraces in unexpected errors handled by `raise_error`
+ matcher (Myron Marston)
Bug fixes
View
13 lib/rspec/matchers/built_in/raise_error.rb
@@ -87,8 +87,19 @@ def expected_error
end
end
+ def format_backtrace(backtrace)
+ formatter = Matchers.configuration.backtrace_formatter
+ formatter.format_backtrace(backtrace)
+ end
+
def given_error
- @actual_error.nil? ? " but nothing was raised" : ", got #{@actual_error.inspect}"
+ return " but nothing was raised" unless @actual_error
+
+ backtrace = format_backtrace(@actual_error.backtrace)
+ [
+ ", got #{@actual_error.inspect} with backtrace:",
+ *backtrace
+ ].join("\n # ")
end
end
end
View
29 lib/rspec/matchers/configuration.rb
@@ -64,6 +64,35 @@ def add_should_and_should_not_to(*modules)
Expectations::Syntax.enable_should(mod)
end
end
+
+ # Sets or gets the backtrace formatter. The backtrace formatter should
+ # implement `#format_backtrace(Array<String>)`. This is used
+ # to format backtraces of errors handled by the `raise_error`
+ # matcher.
+ #
+ # If you are using rspec-core, rspec-core's backtrace formatting
+ # will be used (including respecting the presence or absence of
+ # the `--backtrace` option).
+ #
+ # @overload backtrace_formatter
+ # @return [#format_backtrace] the backtrace formatter
+ # @overload backtrace_formatter=
+ # @param value [#format_backtrace] sets the backtrace formatter
+ attr_writer :backtrace_formatter
+ def backtrace_formatter
+ @backtrace_formatter ||= if defined?(::RSpec::Core::BacktraceFormatter)
+ ::RSpec::Core::BacktraceFormatter
+ else
+ NullBacktraceFormatter
+ end
+ end
+
+ # @api private
+ NullBacktraceFormatter = Module.new do
+ def self.format_backtrace(backtrace)
+ backtrace
+ end
+ end
end
# The configuration object
View
28 spec/rspec/matchers/configuration_spec.rb
@@ -13,6 +13,34 @@ module Matchers
describe Configuration do
let(:config) { Configuration.new }
+ describe "#backtrace_formatter" do
+ let(:original_backtrace) { %w[ clean-me/a.rb other/file.rb clean-me/b.rb ] }
+ let(:cleaned_backtrace) { %w[ other/file.rb ] }
+
+ let(:formatted_backtrace) do
+ config.backtrace_formatter.format_backtrace(original_backtrace)
+ end
+
+ before do
+ RSpec.configuration.stub(:backtrace_clean_patterns) { [/clean-me/] }
+ end
+
+ it "defaults to rspec-core's backtrace formatter when rspec-core is loaded" do
+ expect(config.backtrace_formatter).to be(RSpec::Core::BacktraceFormatter)
+ expect(formatted_backtrace).to eq(cleaned_backtrace)
+ end
+
+ it "defaults to a null formatter when rspec-core is not loaded" do
+ stub_const("RSpec::Core", nil) # so the formatter module is not loaded
+ expect(formatted_backtrace).to eq(original_backtrace)
+ end
+
+ it "can be set to another backtrace formatter" do
+ config.backtrace_formatter = stub(:format_backtrace => ['a'])
+ expect(formatted_backtrace).to eq(['a'])
+ end
+ end
+
context 'on an interpreter that does not provide BasicObject', :unless => defined?(::BasicObject) do
before { RSpec::Expectations::Syntax.disable_should(Delegator) }
View
30 spec/rspec/matchers/raise_error_spec.rb
@@ -51,6 +51,27 @@
lambda { raise RuntimeError, "example message" }.should_not raise_error
}.should fail_with(/expected no Exception, got #<RuntimeError: example message>/)
end
+
+ it 'includes the backtrace of the error that was raised in the error message' do
+ expect {
+ expect { raise "boom" }.not_to raise_error
+ }.to raise_error { |e|
+ backtrace_line = "#{File.basename(__FILE__)}:#{__LINE__ - 2}"
+ e.message.should include("with backtrace", backtrace_line)
+ }
+ end
+
+ it 'formats the backtrace using the configured backtrace formatter' do
+ RSpec::Matchers.configuration.backtrace_formatter.
+ stub(:format_backtrace).
+ and_return("formatted-backtrace")
+
+ expect {
+ expect { raise "boom" }.not_to raise_error
+ }.to raise_error { |e|
+ e.message.should include("with backtrace", "formatted-backtrace")
+ }
+ end
end
describe "should raise_error(message)" do
@@ -77,6 +98,15 @@
lambda {raise NameError.new('blarg')}.should raise_error('blah')
end.should fail_with(/expected Exception with \"blah\", got #<NameError: blarg>/)
end
+
+ it 'includes the backtrace of any other error in the failure message' do
+ expect {
+ expect { raise "boom" }.to raise_error(ArgumentError)
+ }.to raise_error { |e|
+ backtrace_line = "#{File.basename(__FILE__)}:#{__LINE__ - 2}"
+ e.message.should include("with backtrace", backtrace_line)
+ }
+ end
end
describe "should_not raise_error(message)" do
Something went wrong with that request. Please try again.