Rename verifying double callback and expand to partial doubles.

[JonRowe]

This is prep for fixing rspec/rspec-rails#1357 (well actually I think it fixes it but I haven't check from the other side yet so... starting here)

[myronmarston]

These specs show that the callback is called but don't show when it gets called. Is it called when expect_any_instance_of(...).to receive is used? Is it called before implemented gets called? Or after those? It would be nice if the spec would make that explicit (which may require you to move away from using yield_successive_args).

[myronmarston]

Should we also show use of allow_any_instance_of and allow?

[JonRowe]

Do they verify methods? If they do then we should otherwise it doesn't really matter?

[myronmarston]

Do they verify methods? If they do then we should otherwise it doesn't really matter?

Yes, when verify_partial_double is enabled we apply verification to both expect and allow. It would be a bug if we did not. As such I think we should have specs for them here.

[JonRowe]

Yep, ok, I'll add them

[myronmarston]

Given that we plan to move any_instance stuff out of rspec-mocks....I think it's worth asking: is this API (and the semantics of it) what we would want if any_instance wasn't a thing? I want to make sure we don't make our API worse due to any_instance with the fact we plan to move it out in RSpec 4.

[JonRowe]

Yes, I still feel it makes sense to have this callback for all verifying doubles, including partial ones. The only complication I can think of for removing any_instance is that we might have to have a way to access the callbacks from outside of rspec-mocks (it's currently a private api)

[myronmarston]

LGTM. Thoughts on what to do about the app veyor failures? They seem consistent now and I really don't want to keep ignoring.

[JonRowe]

Add some way of filtering libraries and then ignore bundler?

[myronmarston]

The bundler warning has been fixed:

rubygems/bundler@75a8792

So maybe we can force it to use a newer or older version of bundler that doesn't have that warning?

[JonRowe]

See spike at rspec/rspec-support#203, seems we've been pinning it lower

[tmertens]

@myronmarston

we plan to move any_instance stuff out of rspec-mocks

Mind if I ask why this is going away completely? While I understand it is an antipattern, it is sometimes a necessary evil when dealing with legacy code. Is there a mailing list or other place where this discussion is taking or has already taken place so I can get more info?

[myronmarston]

It's not going to go away completely -- we just plan for it to be extracted into a separate gem (something like rspec-mocks-any_instance). Reasons:

• It's a huge PITA to maintain. any_instance is the single biggest source of bugs in all of RSpec, and yet none of the maintainers of RSpec use it. For an example of the gymnastics we have to jump through to get it to work, see Fix any_instance so that it updates already stubbed instances #651...and bear in mind that's not an isolated case; we've had to do that sort of thing repeatedly to get any_instance to work.
• There are all sorts of semantic ambiguities that arise from it. For example, given class A; def foo; end; end and class B < A; def foo; end; end, how should allow_any_instance_of(A).to receive(:foo).and_return(1); B.new.foo do? On the one hand, B is an instance of A, but B is already specializing what foo does and has specifically redefined foo to not do what A#foo does. What if a user does allow_any_instance_of(Object).to receive(:==)? Every object in the system is an instance of Object (besides a handful of BasicObject subclasses that may exist...) and if the user does that they are changing the definition of == on every object which will almost certainly break RSpec and other things. And yet that's what they are telling us to do. Or consider when you use expect_any_instance_of(...).to receive(...).twice -- should both one instance receiving that message twice and two separate instances receiving it once make it pass?
• There are some situations in which we've failed to make any_instance work properly, such as when you prepend a module that defines the stubbed method onto that class.

And as you said, it's an antipattern...on the one hand you're trying to be specific about a specific message being received with specific arguments, but on the other hand you're not being specific about the most important aspect of the method call: the receiver.

We want to continue to support users who rely on this feature by making it available in another gem, without continuing to bear the cost of maintaining it. Hopefully we'll be able to find someone from the community who is willing to step up and maintain it :).

[tmertens]

@myronmarston Thanks for the detailed explanation. I am glad to hear it is not going away, that would make my life very, very painful due to excessive use (by others) of this feature. I would be happy to help with what I can, but I have very little understanding of the rspec internals at this point.