Skip to content

Give a better error message with .at_least(3).times style expectations #255

Merged
merged 7 commits into from Apr 2, 2013

2 participants

@samphippen
RSpec member

Related #252

This is a vastly prototypical implementation. I'm not even sure if it's good or what we want, so I didn't pull the failing specs up to scratch. Any and all comments would be appreciated.

This basically makes the output of this:

  example do
    dbl = double
    dbl.should_receive(:foo).at_least(2).times
    dbl
  end

Look like this

  1) RSpec::Mocks
     Failure/Error: dbl.should_receive(:foo).at_least(2).times
       (Double).foo(any args)
           expected: at least 2 times
           received: 0 times

It does the same thing for at most.

@myronmarston myronmarston and 1 other commented on an outdated diff Mar 31, 2013
lib/rspec/mocks/error_generator.rb
@@ -42,8 +42,8 @@ def raise_similar_message_args_error(expectation, *args_for_multiple_calls)
end
# @private
- def raise_expectation_error(message, expected_received_count, actual_received_count, *args)
- __raise "(#{intro}).#{message}#{format_args(*args)}\n expected: #{count_message(expected_received_count)}\n received: #{count_message(actual_received_count)}"
+ def raise_expectation_error(message, expected_received_count, actual_received_count, expectation_count_type, *args)
+ __raise "(#{intro}).#{message}#{format_args(*args)}\n expected: #{count_message(expected_received_count, expectation_count_type)}\n received: #{count_message(actual_received_count)}"
@myronmarston
RSpec member
myronmarston added a note Mar 31, 2013

I dislike lines that are super long like this...could this be broken up into multiple lines.

(I realize it was already like this, of course).

@samphippen
RSpec member
samphippen added a note Mar 31, 2013

I will break it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@myronmarston myronmarston commented on the diff Mar 31, 2013
lib/rspec/mocks/error_generator.rb
@@ -117,8 +117,9 @@ def arg_list(*args)
args.collect {|arg| arg.respond_to?(:description) ? arg.description : arg.inspect}.join(", ")
end
- def count_message(count)
- return "at least #{pretty_print(count.abs)}" if count < 0
+ def count_message(count, expectation_count_type=nil)
+ return "at least #{pretty_print(count.abs)}" if count < 0 || expectation_count_type == :at_least
+ return "at most #{pretty_print(count)}" if expectation_count_type == :at_most
@myronmarston
RSpec member
myronmarston added a note Mar 31, 2013

I don't really understand the count < 0 business -- do you?

@samphippen
RSpec member
samphippen added a note Mar 31, 2013

No idea

@samphippen
RSpec member
samphippen added a note Mar 31, 2013

@dchelimsky may be able to shed some light?

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

Looks good. Thanks, Sam!

I noticed that the existing specs for should_receive().at_most, etc just expect the particular error class but don't expect anything about the error message. It would be good to update those specs to verify some fragment of the message as well, since it's an important aspect of the behavior.

samphippen added some commits Mar 31, 2013
@samphippen samphippen Make the specs pass
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
04e2809
@samphippen samphippen Break up a monster line in the mocks error generator
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
442775c
@samphippen
RSpec member

@myronmarston which specs are you talking about? The specs in have_received_spec.rb all seem to expect a specific message by regexing.

samphippen added some commits Mar 31, 2013
@samphippen samphippen Add a changelog entry for the new received expectation message
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
80840b1
@samphippen samphippen Fix a cuke related to the new at_least at_most output
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
9671573
@myronmarston myronmarston commented on an outdated diff Mar 31, 2013
spec/rspec/mocks/have_received_spec.rb
@@ -114,7 +114,7 @@ module Mocks
it 'fails when the message was received fewer times' do
expect {
expect(dbl).to have_received(:expected_method).at_least(4).times
- }.to raise_error(/expected: 4 times.*received: 3 times/m) # TODO: better message
+ }.to raise_error(/expected: at least 4 times.*received: 3 times/m) # TODO: better message
@myronmarston
RSpec member
myronmarston added a note Mar 31, 2013

You should remove the TODO (and below as well) since this has now been done :).

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

@myronmarston which specs are you talking about? The specs in have_received_spec.rb all seem to expect a specific message by regexing.

I'm talking about the specs in at_least_spec.rb and at_most_spec.rb.

@myronmarston myronmarston commented on the diff Mar 31, 2013
Changelog.md
@@ -14,6 +14,9 @@ Enhancements:
was received afterwards using the `have_received` matcher.
Note that you must first stub the method or use a null double.
(Joe Ferris and Joël Quenneville)
+* Make `at_least` and `at_most` style receive expectations print that they were
+ expecting at least or at most some number of calls, rather than just the
+ number of calls given in the expectation (Sam Phippen)
@myronmarston
RSpec member
myronmarston added a note Mar 31, 2013

thanks for remembering to update the changelog!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
samphippen added some commits Mar 31, 2013
@samphippen samphippen Make at_least and at_most expect messages instead of classes.
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
b5655e5
@samphippen samphippen Remove some better messages todos
Signed-off-by: Sam Phippen <samphippen@googlemail.com>
1c07b0c
@myronmarston myronmarston merged commit e4190d3 into rspec:master Apr 2, 2013

1 check passed

Details default The Travis build passed
@myronmarston
RSpec member

Thanks, Sam!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.