Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

should_receive'ing a stub'ed method does not work properly #103

Closed
FooBarWidget opened this Issue · 23 comments

5 participants

Hongli Lai Justin Ko preethiramdev David Chelimsky aakash dharmadhikari
Hongli Lai

Test case:

describe "should_receive" do
  it "works properly on stubbed methods" do
    user = mock
    user_class = Object.new
    user_class.stub!(:find).and_return(nil)
    user_class.should_receive(:find).at_least(:once).and_return(user)
    user_class.find(1).should == user
    user_class.find(1).should == user
    user_class.find(1).should == user
  end
end

The user_class stub! call screws up the the should_receive call. I expect user_class.find(1) to return user every time, but what actually happens is that it returns nil after the first call.

Justin Ko

Thanks for reporting this.

preethiramdev

Is this still an open issue? cos I just started working on it. Also should it not respond the way its doing in the example given?

preethiramdev

I mean the call to user_class.find(1).should == user after the first call meets both conditions
1. user_class.stub!(:find).and_return(nil)
2. user_class.should_receive(:find).at_least(:once).and_return(user)
so is it a bug if it stubs the method and returns nil after the first call?

Justin Ko

@preethiramdev I think it makes sense that they should over-ride each other. In the above example, it would be like stub was never called/set. @dchelimsky please confirm.

preethiramdev

@justinko That makes sense though I'm not sure I see a use case when such a sequence of expectations would be set (stub and atleast_once)

Justin Ko

@preethiramdev I agree it should never happen, but I can see how it can easily happen:

before { object.stub(:foo).and_return(1) }

it 'returns 1' do
  object.foo.should eq 1
end

context 'when not returning 1' do
  before { object.should_receive(:foo).and_return(2) }

  it 'returns 2' do
    object.foo.should eq 2
  end
end

The situation can arise if the user becomes sloppy (no offense). Instead of separating the stub and mock, they just attempt to over-ride the stub. I've personally done this myself in my early RSpec days.

preethiramdev

@justinko I can see this happening, so Im going to get started on this and hopefully will have a pull request for you guys soon

David Chelimsky
Owner

I agree this would be a good change, but it would be a breaking change, so we can't just shove it into a 2.x release. We need to offer a configuration option like config.expectations_override_stubs = true, which we can then make the default behavior (and deprecate) in rspec-3.0.

Justin Ko

config.expectations_override_stubs = true but shouldn't stubs override expectations? A stub would clear out any existing expectations on that method (with matching args), and vice versa for expectations defined before a stub.

it '...' do
  foo.should_receive(:bar).and_return(1)
  foo.stub(:bar).and_return(2)
  foo.bar # 2

  foo.should_receive(:bar).and_return(1)
  foo.bar # 1
end
David Chelimsky
Owner

@justinko I see this:

example do
  foo.should_receive(:bar).and_return(1)
  foo.stub(:bar).and_return(2)
end

... and think "return 1 the first time, and return 2 any time after that."

Justin Ko

@dchelimsky I guess I can see expectations always taking precedence. I just worry that methods like at_least will become overly complex. You're changing it from "at_least has been satisfied" to "at_least has been satisfied and will continue to be used". Also, I can't imagine why anyone would want to set an expectation and stub on the same method with the same args...

Hongli Lai

but shouldn't stubs override expectations?

My expectation is that whatever is called last should override the first. A #stub should override a previous #should and a #should should override a previous #stub.

My use case is as follows. Some of my models perform HTTP calls to a service to lookup information. Obviously I don't want it to access the HTTP service to be accessed during unit tests. In 90% of my tests, I want the HTTP call code to be stubbed to return a certain default response, but in 10% of my tests I want a specific HTTP response so I can test related behavior. So I want to #stub the HTTP code with some default values while allowing individual tests to override it with a later #should call.

Justin Ko

@FooBarWidget I agree, and would love to get @myronmarston opinion on all of this.

preethiramdev

@FooBarWidget I agree. @dchelimsky What woould be a usecase for having an expectation and stub on the same method unless the user wanted to override the behaviour?

David Chelimsky
Owner
describe Interactor do
  let(:interactor) { Interactor.new }
  before { interactor.stub(:message) }
  it "sends the first message" do
    interactor.should_receive(:message).with("First message")
    interactor.interact
  end
  it "sends the second message" do
    interactor.should_receive(:message).with("Second message")
    interactor.interact
  end
end
preethiramdev

@dchelimsky Am trying to figure out the preferred way of fixing this and also feel that having the stub or expectation that is defined later to override the stub or expectation that is defined previously rather than having the expectation always take precedance. This seems more intuitive from a user's perspective. Don't you think?

David Chelimsky
Owner

The reported issue is a very specific case involving at_least(:once). To resolve this, we need to have at_least(:once) remove any stubs with the same incoming arguments (which might be none) on the same method. We don't need to introduce a new "last expression wins" policy, have all stubs override all expectations, or vice versa, any of which would be breaking changes that fundamentally change how rspec-mocks works.

preethiramdev

I came across this when working on this issue.

should_receive expectation is created with the same args as a previously defined stub unless args are specified

@mock.stub(:random_call).with(3).and_return("stub")
@mock.should_receive(:random_call).at_least(:once).and_return("expectation")
@mock.random_call
@mock.random_call 3
@mock.rspec_verify

fails with the following message:
received :random_call with unexpected arguments
expected: (3)
got: (no args)

I understand that this is because the expectation is created by cloning the existing stub but was wondering if its supposed to be created that way. Shouldnt the expectation be created with any arguments?

David Chelimsky
Owner

@preethiramdev on the surface that seems reasonable to me. If changing it breaks anything else we'll have to chat :)

preethiramdev

Have submitted a pull request to fix the above mentioned. Don't think it should break anything else :)

preethiramdev

Submitted pull request to close issue 103 based on your previous comments.

You said earlier:The reported issue is a very specific case involving at_least(:once)
@dchelimsky This issue would also appear for any expectations with at_least e.g. at_least(:twice)

David Chelimsky dchelimsky reopened this
aakash dharmadhikari

@dchelimsky is this issue still open?

David Chelimsky
Owner

Nope

David Chelimsky dchelimsky closed this
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.