Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

any_instance and should_not_receive/should_receive usage (from 2.10 to 2.11) #164

Closed
prusswan opened this Issue · 7 comments

4 participants

@prusswan

After upgrading to the latest rspec-rails 2.11, I have a few failing tests involving statements like the following:

some_class.any_instance.should_not_receive(:something)

and after some investigation, I realized there is something not quite right with the tests involved even with rspec 2.10:
they were passing even when both should_not_receive and should_receive are used in the same test!

some_class.any_instance.should_not_receive(:something)
some_class.any_instance.should_receive(:something)

While I know that something has changed with should_receive and the new expectation syntax, I still cannot figure out if the tests are written wrongly, or those methods were bugged in 2.10. Also, are there plans to deprecate the should statements?

@alindeman
Collaborator

Could you write up a complete example of a spec that you expect to pass, but fails (or vice versa)?

@dchelimsky
Owner

Also, are there plans to deprecate the should statements?

Unlikely. The problems we addressed with the expect syntax don't affect most users, and there is a lot of code out there that relies on should.

@prusswan

@alindeman I am trying to create a spec that exactly reproduces the problem (it involves a user model and customized devise registration controller used in a sign up test), but at this point I just want to know if a test with those statements should even pass with any reasonable explanation. If the passing test was bugged in 2.10, I will just fix the version in 2.11 (that is rightly failing now)

@myronmarston

any_instance is convenient for stubbing methods, but once you start using it for mocking, I think it gets confusing very quickly. Consider:

class SendEmailsToActiveUser
  def self.perform
    User.find_each do |user|
      user.send_email if user.active?
    end
  end
end

describe SendEmailsToActiveUser do
  it 'sends emails only to active users' do
    Factory.create(:user, :active => true)
    Factory.create(:user, :active => false)

    User.any_instance.should_receive(:send_email)
    User.any_instance.should_not_receive(:send_email)

    SendEmailsToActiveUser.perform
  end
end

Here I've got any_instance.should_receive and any_instance.should_not_receive in the same test. It's ambiguous what it means, but it can be argued that this test should pass, because there is at least one user instance that did receive :send_email and at least one user instance that did not receive :send_email. Or is should_not_receive supposed to be for all instances?

In short, I think that any_instance.should_not_receive is ambiguous. Even if we get it working "right" (for some definition of "right"), I think it's best to avoid it because it's open to alternate interpretations.

Also, are there plans to deprecate the should statements?

There's a discussion about a new syntax (to go along with the expectation syntax) in #153. As @dchelimsky said, even if we do introduce a new syntax, it's doubtful we'd ever remove the existing syntax entirely.

@prusswan

@myronmarston that actually makes a lot of sense. So as a start I should just bind should_not_receive to a specific user instance? or is there a better way of doing this with the new syntax?

@myronmarston

So as a start I should just bind should_not_receive to a specific user instance?

I generally avoid any_instance and do all my mocking on specific instances. There's far less ambiguity and edge cases. For stubbing I think any_instance is useful and doesn't have the ambiguities.

Still, we need to have defined behavior for any_instance.should_not_receive and I'm not sure what it should be, so it's good that you brought this up :).

@myronmarston

or is there a better way of doing this with the new syntax?

The new syntax is just a discussion at this point. No code has been written for it yet.

@prusswan prusswan 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.