Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #382 from rspec/warn_when_overriding_implementation

Don't silently ignore arbitrary method expectations when combining them with 'and_call_original'
  • Loading branch information...
commit f434af58f87937aa09c44b4f50719aaa836bf6b0 2 parents 07f96d4 + 659b039
@JonRowe JonRowe authored
View
18 lib/rspec/mocks/deprecation.rb
@@ -1,18 +0,0 @@
-module RSpec
- module Mocks
- module Deprecation
- # @private
- #
- # Used internally to print deprecation warnings
- def deprecate(deprecated, options={})
- message = "DEPRECATION: #{deprecated} is deprecated."
- message << " Use #{options[:replacement]} instead." if options[:replacement]
- message << " Called from #{CallerFilter.first_non_rspec_line}."
- warn message
- end
- end
- end
-
- extend(Mocks::Deprecation) unless respond_to?(:deprecate)
-end
-
View
2  lib/rspec/mocks/framework.rb
@@ -3,7 +3,7 @@
# object in the system.
require 'rspec/mocks/caller_filter' unless defined?(::RSpec::CallerFilter)
-require 'rspec/mocks/deprecation'
+require 'rspec/mocks/warnings'
require 'rspec/mocks/instance_method_stasher'
require 'rspec/mocks/method_double'
require 'rspec/mocks/argument_matchers'
View
8 lib/rspec/mocks/message_expectation.rb
@@ -149,6 +149,9 @@ def and_call_original
if RSpec::Mocks::TestDouble === @method_double.object
@error_generator.raise_only_valid_on_a_partial_mock(:and_call_original)
else
+ if implementation.inner_action
+ RSpec.warning("You're overriding a previous implementation for this stub")
+ end
@implementation = AndCallOriginalImplementation.new(@method_double.original_method)
@yield_receiver_to_implementation_block = false
end
@@ -501,6 +504,7 @@ def initial_implementation_action=(action)
end
def inner_implementation_action=(action)
+ RSpec.warning("You're overriding a previous implementation for this stub") if implementation.inner_action
implementation.inner_action = action if action
end
@@ -597,6 +601,10 @@ def present?
true
end
+ def inner_action
+ true
+ end
+
def call(*args, &block)
@method.call(*args, &block)
end
View
37 lib/rspec/mocks/warnings.rb
@@ -0,0 +1,37 @@
+module RSpec
+
+ # We don't redefine the deprecation helpers
+ # when they already exist (defined by rspec-core etc)
+ unless respond_to?(:deprecate)
+
+ # @private
+ #
+ # Used internally to print deprecation warnings
+ def self.deprecate(deprecated, options = {})
+ warn_with "DEPRECATION: #{deprecated} is deprecated.", options
+ end
+
+ end
+
+ # We don't redefine the warnings helpers
+ # when they already exist (defined by rspec-core etc)
+ unless respond_to?(:warning) && respond_to?(:warn_with)
+
+ # @private
+ #
+ # Used internally to print deprecation warnings
+ def self.warning(text, options={})
+ warn_with "WARNING: #{text}.", options
+ end
+
+ # @private
+ #
+ # Used internally to longer warnings
+ def self.warn_with(message, options = {})
+ message << " Use #{options[:replacement]} instead." if options[:replacement]
+ message << " Called from #{CallerFilter.first_non_rspec_line}."
+ ::Kernel.warn message
+ end
+
+ end
+end
View
14 spec/rspec/mocks/and_call_original_spec.rb
@@ -22,7 +22,7 @@ def self.new_instance
let(:instance) { klass.new }
it 'passes the received message through to the original method' do
- instance.should_receive(:meth_1).and_call_original
+ expect(instance).to receive(:meth_1).and_call_original
expect(instance.meth_1).to eq(:original)
end
@@ -33,19 +33,25 @@ def self.new_instance
end
it 'passes args and blocks through to the original method' do
- instance.should_receive(:meth_2).and_call_original
+ expect(instance).to receive(:meth_2).and_call_original
value = instance.meth_2(:submitted_arg) { |a, b| [a, b] }
expect(value).to eq([:submitted_arg, :additional_yielded_arg])
end
it 'errors when you pass through the wrong number of args' do
- instance.stub(:meth_1).and_call_original
- instance.stub(:meth_2).and_call_original
+ expect(instance).to receive(:meth_1).and_call_original
+ expect(instance).to receive(:meth_2).twice.and_call_original
expect { instance.meth_1 :a }.to raise_error ArgumentError
expect { instance.meth_2 {} }.to raise_error ArgumentError
expect { instance.meth_2(:a, :b) {} }.to raise_error ArgumentError
end
+ it 'warns when you override an existing implementation' do
+ expect(RSpec).to receive(:warning).with /overriding a previous implementation/
+ expect(instance).to receive(:meth_1) { true }.and_call_original
+ instance.meth_1
+ end
+
context "for singleton methods" do
it 'works' do
def instance.foo; :bar; end
View
7 spec/rspec/mocks/combining_implementation_instructions_spec.rb
@@ -159,6 +159,7 @@ def verify_combined_implementation
end
it 'allows the inner implementation block to be overriden' do
+ allow(RSpec).to receive(:warning)
dbl = double
stubbed_double = dbl.stub(:foo)
@@ -169,6 +170,11 @@ def verify_combined_implementation
expect(dbl.foo(:arg)).to eq(:at_least_block)
end
+ it 'warns when the inner implementation block is overriden' do
+ expect(RSpec).to receive(:warning).with /overriding a previous implementation/
+ double.stub(:foo).with(:arg) { :with_block }.at_least(:once) { :at_least_block }
+ end
+
it 'can combine and_call_original, with, and_return' do
obj = Struct.new(:value).new('original')
obj.stub(:value).and_call_original
@@ -178,6 +184,7 @@ def verify_combined_implementation
end
it 'raises an error if `and_call_original` is followed by any other instructions' do
+ allow(RSpec).to receive(:warning)
dbl = [1, 2, 3]
stubbed = dbl.stub(:size)
stubbed.and_call_original
Please sign in to comment.
Something went wrong with that request. Please try again.