From d49cfcae1b294e12a05e06a5612cb8ebb22a7df1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Guti=C3=A9rrez?= Date: Wed, 7 Oct 2015 21:48:10 -0500 Subject: [PATCH] Adding support to allow_nil option for delegate_method matcher --- .../independent/delegate_method_matcher.rb | 101 ++++++++--- spec/acceptance/independent_matchers_spec.rb | 4 +- .../delegate_method_matcher_spec.rb | 171 ++++++++++++++++-- 3 files changed, 234 insertions(+), 42 deletions(-) diff --git a/lib/shoulda/matchers/independent/delegate_method_matcher.rb b/lib/shoulda/matchers/independent/delegate_method_matcher.rb index a79cfc18..b94b156d 100644 --- a/lib/shoulda/matchers/independent/delegate_method_matcher.rb +++ b/lib/shoulda/matchers/independent/delegate_method_matcher.rb @@ -162,10 +162,11 @@ module Shoulda @delegate_method = @delegating_method @delegate_object = Doublespeak::ObjectDouble.new - @delegated_arguments = [] - @delegate_object_reader_method = nil + @context = nil @subject = nil - @subject_double_collection = nil + @delegate_object_reader_method = nil + @delegated_arguments = [] + @expects_to_allow_nil_delegate_object = false end def in_context(context) @@ -180,11 +181,13 @@ module Shoulda subject_has_delegating_method? && subject_has_delegate_object_reader_method? && - subject_delegates_to_delegate_object_correctly? + subject_delegates_to_delegate_object_correctly? && + subject_handles_nil_delegate_object? end def description - string = "delegate #{formatted_delegating_method_name} to " + + string = + "delegate #{formatted_delegating_method_name} to the " + "#{formatted_delegate_object_reader_method_name} object" if delegated_arguments.any? @@ -195,6 +198,12 @@ module Shoulda string << " as #{formatted_delegate_method}" end + if expects_to_allow_nil_delegate_object? + string << ", allowing " + string << formatted_delegate_object_reader_method_name + string << " to return nil" + end + string end @@ -220,6 +229,11 @@ module Shoulda self end + def allow_nil + @expects_to_allow_nil_delegate_object = true + self + end + def build_delegating_method_prefix(prefix) case prefix when true, nil then delegate_object_reader_method @@ -228,14 +242,29 @@ module Shoulda end def failure_message - "Expected #{class_under_test} to #{description}\n" + - "Method calls sent to " + - "#{formatted_delegate_object_reader_method_name(include_module: true)}:" + - formatted_calls_on_delegate_object + message = "Expected #{class_under_test} to #{description}.\n\n" + + if failed_to_allow_nil_delegate_object? + message << formatted_delegating_method_name(include_module: true) + message << ' did delegate to ' + message << formatted_delegate_object_reader_method_name + message << ' when it was non-nil, but it failed to account ' + message << 'for when ' + message << formatted_delegate_object_reader_method_name + message << ' *was* nil.' + else + message << 'Method calls sent to ' + message << formatted_delegate_object_reader_method_name( + include_module: true, + ) + message << ": #{formatted_calls_on_delegate_object}" + end + + Shoulda::Matchers.word_wrap(message) end def failure_message_when_negated - "Expected #{class_under_test} not to #{description}, but it did" + "Expected #{class_under_test} not to #{description}, but it did." end protected @@ -246,7 +275,6 @@ module Shoulda :delegating_method, :method, :delegate_method, - :subject_double_collection, :delegate_object, :delegate_object_reader_method @@ -270,6 +298,10 @@ module Shoulda end end + def expects_to_allow_nil_delegate_object? + @expects_to_allow_nil_delegate_object + end + def formatted_delegate_method(options = {}) formatted_method_name_for(delegate_method, options) end @@ -329,11 +361,7 @@ module Shoulda end def subject_delegates_to_delegate_object_correctly? - register_subject_double_collection - - Doublespeak.with_doubles_activated do - subject.public_send(delegating_method, *delegated_arguments) - end + call_delegating_method_with_delegate_method_returning(delegate_object) if delegated_arguments.any? delegate_object_received_call_with_delegated_arguments? @@ -342,13 +370,44 @@ module Shoulda end end - def register_subject_double_collection + def subject_handles_nil_delegate_object? + @subject_handled_nil_delegate_object = + if expects_to_allow_nil_delegate_object? + begin + call_delegating_method_with_delegate_method_returning(nil) + true + rescue Module::DelegationError + false + rescue NoMethodError => error + if error.message =~ /undefined method `#{delegate_method}' for nil:NilClass/ + false + else + raise error + end + end + else + true + end + end + + def failed_to_allow_nil_delegate_object? + expects_to_allow_nil_delegate_object? && + !@subject_handled_nil_delegate_object + end + + def call_delegating_method_with_delegate_method_returning(value) + register_subject_double_collection_to(value) + + Doublespeak.with_doubles_activated do + subject.public_send(delegating_method, *delegated_arguments) + end + end + + def register_subject_double_collection_to(returned_value) double_collection = Doublespeak.double_collection_for(subject.singleton_class) double_collection.register_stub(delegate_object_reader_method). - to_return(delegate_object) - - @subject_double_collection = double_collection + to_return(returned_value) end def calls_to_delegate_method @@ -363,7 +422,7 @@ module Shoulda string = "" if calls_on_delegate_object.any? - string << "\n" + string << "\n\n" calls_on_delegate_object.each_with_index do |call, i| name = call.method_name args = call.args.map { |arg| arg.inspect }.join(', ') diff --git a/spec/acceptance/independent_matchers_spec.rb b/spec/acceptance/independent_matchers_spec.rb index 0db4aee7..8f5062d9 100644 --- a/spec/acceptance/independent_matchers_spec.rb +++ b/spec/acceptance/independent_matchers_spec.rb @@ -56,7 +56,7 @@ describe 'shoulda-matchers has independent matchers, specifically delegate_metho expect(result).to indicate_number_of_tests_was_run(1) expect(result).to have_output( - 'Courier should delegate #deliver to #post_office object' + 'Courier should delegate #deliver to the #post_office object' ) end @@ -119,7 +119,7 @@ describe 'shoulda-matchers has independent matchers, specifically delegate_metho expect(result).to indicate_number_of_tests_was_run(1) expect(result).to have_output( - /Courier\s+should delegate #deliver to #post_office object/ + /Courier\s+should delegate #deliver to the #post_office object/ ) end end diff --git a/spec/unit/shoulda/matchers/independent/delegate_method_matcher_spec.rb b/spec/unit/shoulda/matchers/independent/delegate_method_matcher_spec.rb index 7fb29b7b..80e952ec 100644 --- a/spec/unit/shoulda/matchers/independent/delegate_method_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/independent/delegate_method_matcher_spec.rb @@ -8,7 +8,7 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do context 'without any qualifiers' do it 'states that it should delegate method to the right object' do matcher = delegate_method(:method_name).to(:delegate) - message = 'delegate #method_name to #delegate object' + message = 'delegate #method_name to the #delegate object' expect(matcher.description).to eq message end @@ -17,7 +17,7 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do context 'qualified with #as' do it 'states that it should delegate method to the right object and method' do matcher = delegate_method(:method_name).to(:delegate).as(:alternate) - message = 'delegate #method_name to #delegate object as #alternate' + message = 'delegate #method_name to the #delegate object as #alternate' expect(matcher.description).to eq message end @@ -27,7 +27,7 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do it 'states that it should delegate method to the right object with right argument' do matcher = delegate_method(:method_name).to(:delegate). with_arguments(:foo, bar: [1, 2]) - message = 'delegate #method_name to #delegate object passing arguments [:foo, {:bar=>[1, 2]}]' + message = 'delegate #method_name to the #delegate object passing arguments [:foo, {:bar=>[1, 2]}]' expect(matcher.description).to eq message end @@ -53,7 +53,7 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do matcher = delegate_method(:hello).to(:country).with_prefix expect(matcher.description). - to eq('delegate #country_hello to #country object as #hello') + to eq('delegate #country_hello to the #country object as #hello') end end end @@ -77,7 +77,7 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do matcher = delegate_method(:hello).to(:country).with_prefix(true) expect(matcher.description). - to eq('delegate #country_hello to #country object as #hello') + to eq('delegate #country_hello to the #country object as #hello') end end end @@ -98,7 +98,7 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do matcher = delegate_method(:hello).to(:country).with_prefix('county') expect(matcher.description). - to eq('delegate #county_hello to #country object as #hello') + to eq('delegate #county_hello to the #country object as #hello') end end end @@ -112,14 +112,14 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do matcher = delegate_method(:method_name).to(:delegate) expect(matcher.description). - to eq 'delegate .method_name to .delegate object' + to eq 'delegate .method_name to the .delegate object' end end context 'qualified with #as' do it 'states that it should delegate method to the right object and method' do matcher = delegate_method(:method_name).to(:delegate).as(:alternate) - message = 'delegate .method_name to .delegate object as .alternate' + message = 'delegate .method_name to the .delegate object as .alternate' expect(matcher.description).to eq message end @@ -129,7 +129,7 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do it 'states that it should delegate method to the right object with right argument' do matcher = delegate_method(:method_name).to(:delegate). with_arguments(:foo, bar: [1, 2]) - message = 'delegate .method_name to .delegate object passing arguments [:foo, {:bar=>[1, 2]}]' + message = 'delegate .method_name to the .delegate object passing arguments [:foo, {:bar=>[1, 2]}]' expect(matcher.description).to eq message end @@ -178,7 +178,8 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do it 'rejects with the correct failure message' do post_office = PostOffice.new message = [ - 'Expected PostOffice to delegate #deliver_mail to #mailman object', + 'Expected PostOffice to delegate #deliver_mail to the #mailman object.', + '', 'Method calls sent to PostOffice#mailman: (none)' ].join("\n") @@ -191,7 +192,8 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do context 'when the subject is a class' do it 'uses the proper syntax for class methods in errors' do message = [ - 'Expected PostOffice to delegate .deliver_mail to .mailman object', + 'Expected PostOffice to delegate .deliver_mail to the .mailman object.', + '', 'Method calls sent to PostOffice.mailman: (none)' ].join("\n") @@ -225,7 +227,7 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do context 'negating the matcher' do it 'rejects with the correct failure message' do post_office = PostOffice.new - message = 'Expected PostOffice not to delegate #deliver_mail to #mailman object, but it did' + message = 'Expected PostOffice not to delegate #deliver_mail to the #mailman object, but it did.' expect { expect(post_office).not_to delegate_method(:deliver_mail).to(:mailman) @@ -283,7 +285,7 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do context 'negating the matcher' do it 'rejects with the correct failure message' do post_office = PostOffice.new - message = 'Expected PostOffice not to delegate #deliver_mail to #mailman object passing arguments ["221B Baker St.", {:hastily=>true}], but it did' + message = 'Expected PostOffice not to delegate #deliver_mail to the #mailman object passing arguments ["221B Baker St.", {:hastily=>true}], but it did.' expect { expect(post_office). @@ -299,8 +301,11 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do it 'rejects with the correct failure message' do post_office = PostOffice.new message = [ - 'Expected PostOffice to delegate #deliver_mail to #mailman object passing arguments ["123 Nowhere Ln."]', + 'Expected PostOffice to delegate #deliver_mail to the #mailman object', + 'passing arguments ["123 Nowhere Ln."].', + '', 'Method calls sent to PostOffice#mailman:', + '', '1) deliver_mail("221B Baker St.", {:hastily=>true})' ].join("\n") @@ -338,7 +343,7 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do context 'negating the assertion' do it 'rejects with the correct failure message' do post_office = PostOffice.new - message = 'Expected PostOffice not to delegate #deliver_mail to #mailman object as #deliver_mail_and_avoid_dogs, but it did' + message = 'Expected PostOffice not to delegate #deliver_mail to the #mailman object as #deliver_mail_and_avoid_dogs, but it did.' expect { expect(post_office). @@ -354,8 +359,11 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do it 'rejects with the correct failure message' do post_office = PostOffice.new message = [ - 'Expected PostOffice to delegate #deliver_mail to #mailman object as #watch_tv', + 'Expected PostOffice to delegate #deliver_mail to the #mailman object as', + '#watch_tv.', + '', 'Method calls sent to PostOffice#mailman:', + '', '1) deliver_mail_and_avoid_dogs()' ].join("\n") @@ -401,7 +409,9 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do end message = [ - 'Expected Person to delegate #country_hello to #country object as #hello', + 'Expected Person to delegate #country_hello to the #country object as', + '#hello.', + '', 'Method calls sent to Person#country: (none)' ].join("\n") @@ -449,7 +459,9 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do end message = [ - 'Expected Person to delegate #country_hello to #country object as #hello', + 'Expected Person to delegate #country_hello to the #country object as', + '#hello.', + '', 'Method calls sent to Person#country: (none)' ].join("\n") @@ -499,7 +511,9 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do end message = [ - 'Expected Person to delegate #county_hello to #country object as #hello', + 'Expected Person to delegate #county_hello to the #country object as', + '#hello.', + '', 'Method calls sent to Person#country: (none)' ].join("\n") @@ -514,4 +528,123 @@ describe Shoulda::Matchers::Independent::DelegateMethodMatcher do end end end + + context 'qualified with #allow_nil' do + context 'when using delegate from Rails' do + context 'when delegations were defined with :allow_nil' do + it 'accepts' do + define_class('Person') do + delegate :hello, to: :country, allow_nil: true + def country; end + end + + person = Person.new + + expect(person).to delegate_method(:hello).to(:country).allow_nil + end + end + + context 'when delegations were not defined with :allow_nil' do + it 'rejects with the correct failure message' do + define_class('Person') do + delegate :hello, to: :country + def country; end + end + + person = Person.new + + message = <<-MESSAGE +Expected Person to delegate #hello to the #country object, allowing +#country to return nil. + +Person#hello did delegate to #country when it was non-nil, but it failed +to account for when #country *was* nil. + MESSAGE + + expectation = lambda do + expect(person).to delegate_method(:hello).to(:country).allow_nil + end + + expect(&expectation).to fail_with_message(message) + end + end + end + + context 'when using Forwardable' do + context 'when the delegate object is nil' do + it 'rejects with the correct failure message' do + define_class('Person') do + extend Forwardable + + def_delegators :country, :hello + + def country; end + end + + person = Person.new + + message = <<-MESSAGE +Expected Person to delegate #hello to the #country object, allowing +#country to return nil. + +Person#hello did delegate to #country when it was non-nil, but it failed +to account for when #country *was* nil. + MESSAGE + + expectation = lambda do + expect(person).to delegate_method(:hello).to(:country).allow_nil + end + + expect(&expectation).to fail_with_message(message) + end + end + end + + context 'when delegating manually' do + context 'when the delegating method accounts for the delegate object being nil' do + it 'accepts' do + define_class('Person') do + def country; end + + def hello + return unless country + country.hello + end + end + + person = Person.new + + expect(person).to delegate_method(:hello).to(:country).allow_nil + end + end + + context 'when the delegating method does not account for the delegate object being nil' do + it 'rejects with the correct failure message' do + define_class('Person') do + def country; end + + def hello + country.hello + end + end + + person = Person.new + + message = <<-MESSAGE +Expected Person to delegate #hello to the #country object, allowing +#country to return nil. + +Person#hello did delegate to #country when it was non-nil, but it failed +to account for when #country *was* nil. + MESSAGE + + expectation = lambda do + expect(person).to delegate_method(:hello).to(:country).allow_nil + end + + expect(&expectation).to fail_with_message(message) + end + end + end + end end