Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass the instance to any instance stubs. #351

Merged
5 changes: 5 additions & 0 deletions lib/rspec/mocks/any_instance/expectation_chain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ class PositiveExpectationChain < ExpectationChain
def create_message_expectation_on(instance)
proxy = ::RSpec::Mocks.proxy_for(instance)
expected_from = IGNORED_BACKTRACE_LINE
if @expectation_args.last.is_a? Hash
@expectation_args.last[:is_any_instance_expectation] = true
else
@expectation_args << {:is_any_instance_expectation => true}
end
proxy.add_message_expectation(expected_from, *@expectation_args, &@expectation_block)
end

Expand Down
3 changes: 3 additions & 0 deletions lib/rspec/mocks/any_instance/recorder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ def observe!(method_name)
@klass.__send__(:define_method, method_name) do |*args, &blk|
klass = ::RSpec::Mocks.method_handle_for(self, method_name).owner
::RSpec::Mocks.any_instance_recorder_for(klass).playback!(self, method_name)
if ::RSpec::Mocks.configuration.pass_instance_to_any_instance_stubs
args.unshift(self)
end
self.__send__(method_name, *args, &blk)
end
end
Expand Down
8 changes: 8 additions & 0 deletions lib/rspec/mocks/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ def add_stub_and_should_receive_to(*modules)
end
end

def pass_instance_to_any_instance_stubs
@pass_instance_to_any_instance_stubs ||= false
end

def pass_instance_to_any_instance_stubs=(arg)
@pass_instance_to_any_instance_stubs = arg
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yield_instance_from_any_instance_implementation_blocks is a better name. What do you think?


def syntax=(values)
if Array(values).include?(:expect)
Syntax.enable_expect
Expand Down
5 changes: 5 additions & 0 deletions lib/rspec/mocks/message_expectation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def initialize(error_generator, expectation_ordering, expected_from, method_doub
@args_to_yield = []
@failed_fast = nil
@eval_context = nil
@is_any_instance_expectation = opts[:is_any_instance_expectation]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't used anymore, right?


@implementation = Implementation.new
self.inner_implementation_action = implementation_block
Expand Down Expand Up @@ -168,6 +169,10 @@ def and_yield(*args, &block)

# @private
def matches?(message, *args)
args = args.dup
if @is_any_instance_expectation && ::RSpec::Mocks.configuration.pass_instance_to_any_instance_stubs
args.shift
end
@message == message && @argument_list_matcher.args_match?(*args)
end

Expand Down
65 changes: 65 additions & 0 deletions spec/rspec/mocks/any_instance_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,71 @@ def foo; end
end
end

context "passing self" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "passing self" is a bit of a confusing/ambiguous doc string -- there are many selfs involved here, and it's not clear which you mean. Maybe "passing the receiver to the implementation block" is better?

context "when configured to pass the instance" do
before(:each) do
@orig_pass = RSpec::Mocks.configuration.pass_instance_to_any_instance_stubs
RSpec::Mocks.configuration.pass_instance_to_any_instance_stubs = true
end

after(:each) do
RSpec::Mocks.configuration.pass_instance_to_any_instance_stubs = @orig_pass
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than putting this here, what do you think about creating a with isolated configuration shared context? To me, isolating configuration seems like a cross-cutting concern we'll want to use elsewhere. Also, I recommend managing the isolation of the configuration by making that shared example group clear the RSpec::Mocks::Configuration instance -- that way, it isolates any config change, as opposed to just this one config option. Thoughts?


describe "an any instance stub" do
it "receives the instance" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about it "passes the instance as the first arg of the implementation block"?

klass = Struct.new(:science)
instance = klass.new
klass.any_instance.stub(:bees) { |*args| expect(args.first).to eq(instance) }
instance.bees
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could benefit from my suggested alternative below...

end
end

describe "an any instance expectation" do
it "doesn't effect with" do
klass = Struct.new(:science)
instance = klass.new
klass.any_instance.should_receive(:bees).with(:sup)
instance.bees(:sup)
end

it "passes the instance" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about it "passes the instance as the first arg of the implementation block"?

klass = Struct.new(:science)
instance = klass.new
klass.any_instance.should_receive(:bees).with(:sup) { |*args| expect(args.first).to eq(instance) }
instance.bees(:sup)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested alternative:

instance = klass.new

expect { |b|
  klass.any_instance.should_receive(:bees).with(:sup, &b)
  instance.bees(:sup)
}.to yield_with_args(instance, :sup)

Reasons I like this better:

  • If there was a bug that caused the implementation block to not be called in this case, the spec would silently fail (since the expectation you've set wouldn't even run in that case).
  • It matters that the other args are included after the instance, so this specs that as well.
  • It uses a handy matcher from rspec-expectations :).

end
end
end

context "when configured not to pass the instance" do
before(:each) do
@orig_pass = RSpec::Mocks.configuration.pass_instance_to_any_instance_stubs
RSpec::Mocks.configuration.pass_instance_to_any_instance_stubs = false
end

after(:each) do
RSpec::Mocks.configuration.pass_instance_to_any_instance_stubs = @orig_pass
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you extract the shared context I suggested above, it could be used here as well :).


describe "an any instance stub" do
it "does not receive the instance" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about it "does not pass the instance to the implementation block"?

klass = Struct.new(:science)
instance = klass.new
klass.any_instance.stub(:bees) { |*args| expect(args).to be_empty }
instance.bees
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could benefit from my suggested alternative above...

end

it "gets data from with correctly" do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doc string doesn't make much sense to me...what are you trying to test here?

klass = Struct.new(:science)
instance = klass.new
klass.any_instance.should_receive(:bees).with(:faces)
instance.bees(:faces)
end
end
end
end

context 'when used in conjunction with a `dup`' do
it "doesn't cause an infinite loop" do
pending "This intermittently fails on JRuby" if RUBY_PLATFORM == 'java'
Expand Down