Move rspec-fire functionality to rspec-mocks

[mikegehard]

Thanks for writing rspec-fire

With the addition of constant stubbing to rspec-mocks(http://myronmars.to/n/dev-blog/2012/06/constant-stubbing-in-rspec-2-11), I would like to start the conversation about moving some of the other functionality into rspec-mocks.

I really love the ability to practice a mockist style of xDD and know that when I refactor a method that my mocks aren't out of sync with reality and think that others would practice more mockists style xDD with confidence if they had this functionality.

On suggestion of @myronmarston I would like to see what everyone has to say.

[myronmarston]

I don't have a strong opinion on the matter--but if there is community support for getting rspec-fire's functionality into rspec-mocks, and @xaviershay is on board with that, then I'd be in favor of it.

/cc @dchelimsky @justinko @patmaddox @spicycode @alindeman

[dchelimsky]

There is a point in the TDD workflow where you mock a method that doesn't yet exist on the collaborator. Does rspec-fire allow me to do that? If not, that's a deal breaker for me. If there's away, I'm listening.

[mikegehard]

From the README:

Another solution, that rspec-fire adopts, is a more relaxed version that only checks that the methods exist if the doubled class has already been loaded. No checking will happen when running the spec in isolation, but when run in the context of the full app (either as a full spec run or by explicitly preloading collaborators on the command line) a failure will be triggered if an invalid method is being stubbed.

@dchelimsky, is this the answer you were looking for? :-) Seems similar to what a compiler would do in something like Java or Scala when running the full suite.

[myronmarston]

To follow up to what @msgehard said, with rspec-fire, there's a few ways to mock a method that doesn't yet exist on the collaborator:

• Use a normal rspec-mocks double (stub, mock or double) rather than fire_double or fire_class_double. rspec-fire adds a new type of double but doesn't change the behavior of the existing test doubles provided by rspec-mocks. Later on, when you have defined the method on the collaborator, you can change the double to a fire_double, and gain the added benefit of the interface verification.
• If the concrete production collaborator class has not yet been defined, you can use fire_double and it will act just like a regular double. Later on, when you do defined the named constant, you'll need to define the mocked method as well, or the test that uses the fire double will start to fail if it runs with the constant loaded.
• If you plan to add the new method to the collaborator before committing the code, and your tests are being run in isolation, you can use a fire double just fine. When you run your entire test suite it'll presumably load everything, and trigger a "mocking unimplemented method" failure, which can help guide you to implement the mocked method on the collaborator before committing the changes.

The important thing here is that rspec-fire's interface checking is completely opt-in; if you don't want it, then you don't need to opt-in.

If/when we do port this to rspec-mocks, I imagine the API would be a bit different; fire_double makes sense currently since the library is called rspec-fire, but in the context of rspec-mocks, it would be a bit strange. Maybe it would be a method like as_null_object that changes how a rspec-mocks test double behaves? Something like:

collaborator = double("MyCollaboratorClass").with_interface_check

(I'm not totally happy with with_interface_check as the method name, but you get the point).

[xaviershay]

I don't have a strong opinion either way. It's easier to keep everything up-to-date and in sync if it is in rspec-mocks.

[alindeman]

I think a #with_interface_check method on doubles would be a compelling feature within rspec-mocks itself.

I also think the name #with_interface_check needs some iteration, though I don't have any good suggestions right now. Naming is hard :)

[alindeman]

We'd have to think about how to handle class-level interfaces vs. instance-level interfaces. Would double("MyCollaboratorClass").with_interface_check (or whatever the name is) verify against class-level methods or instance methods?

I assume we'd need want to be able to check against either interface, but we'd have to be careful that the API we expose to do it isn't terrible :) Designing good APIs is hard.

[myronmarston]

We'd have to think about how to handle class-level interfaces vs. instance-level interfaces. Would double("MyCollaboratorClass").with_interface_check (or whatever the name is) verify against class-level methods or instance methods?

In rspec fire, you distinguish by declaring a different type of double: fire_double vs fire_class_double. However, I know at one point that @xaviershay and I discussed that fire_class_double is a bit of a misnomer; the named constant need not be a class--it really supports verifying singleton methods on any constant.

Maybe double("MyCollaboratorClass").verify_messages(:instance) vs double("MyCollaboratorClass").verify_messages(:singleton)?

[justinko]

#verify_instance_methods
#verify_singleton_methods
#verify_methods (verifies instance and singleton)

The above is in alignment with Ruby (sans "verify_").

Sent from my iPhone

On Jun 30, 2012, at 7:49 PM, Myron Marstonreply@reply.github.com wrote:

We'd have to think about how to handle class-level interfaces vs. instance-level interfaces. Would double("MyCollaboratorClass").with_interface_check (or whatever the name is) verify against class-level methods or instance methods?

In rspec fire, you distinguish by declaring a different type of double: fire_double vs fire_class_double. However, I know at one point that @xaviershay and I discussed that fire_class_double is a bit of a misnomer; the named constant need not be a class--it really supports verifying singleton methods on any constant.

Maybe double("MyCollaboratorClass").verify_messages(:instance) vs double("MyCollaboratorClass").verify_messages(:singleton)?

---

Reply to this email directly or view it on GitHub:
#18 (comment)

[alindeman]

I feel like the term interface is important; I think it'd be nice if it showed up in the API. Thoughts? Or is that too un-Ruby?

My thoughts: while Ruby doesn't have formalized interfaces like Java or C#, interface is a generic enough term for an API an object exposes. It'd be consistent with the "I" in [SOLID](http://en.wikipedia.org/wiki/SOLID_(object-oriented_design\)) (which test doubles can help improve!) and other literature on TDD.

[myronmarston]

#verify_methods (verifies instance and singleton)

this doesn't really make sense...a single collaborator is not going to be both a class and an instance of itself.

[mikegehard]

Thanks all for the discussions on this. Looking forward to getting this into rpsec-mocks.

What if there was a configuration setting to toggle the "interface checking" on/off instead of having to declare it on each double? This config setting would default to off.

It seems to me that if you wanted this functionality, you would want it across the board similar to using a compiler. The downside of this would be you could get some "not implemented" failures if you didn't run tests in isolation. In this case you could simply implement the empty methods and add pending specs as reminders that you need to implement those methods.

Along the lines of checking both instance and singleton methods, since we know that a collaborator will either represent a class or an instance of the class, we could check both places. Not sure how performant this is but it should be as simple as checking on the methods of a class.

[myronmarston]

What if there was a configuration setting to toggle the "interface checking" on/off instead of having to declare it on each double? This config setting would default to off.

I actually really like how it's opt-in on a per-test-double basis. Making it on/off globally makes it harder to do things like what @dchelimsky mentioned, where you need to mock a new method as part of designing the interface of the collaborator.

Along the lines of checking both instance and singleton methods, since we know that a collaborator will either represent a class or an instance of the class, we could check both places. Not sure how performant this is but it should be as simple as checking on the methods of a class.

This sounds like a really bad idea to me. I feel like newcomers to rspec-mocks (and ruby) already have a hard enough time with the singleton vs instance method distinction (how many times have we seen mailing list or stack overflow questions about "rspec-mocks isn't working right..." when it's actually that they were mocking a class method rather than an instance method or vice versa?) and making this "magically" handle both would lead to increased confusion, IMO. In addition, if you check both the singleton and instance interfaces of the named constant, then you could get in situations where it allows a test to mock one singleton method and one instance method--but the real collaborator isn't going to work in the same way.

[xaviershay]

I actually really like how it's opt-in on a per-test-double basis. Making it on/off globally makes it harder to do things like what @dchelimsky mentioned, where you need to mock a new method as part of designing the interface of the collaborator.

Not necessarily, you can do fire_double('SomeBogusClass') to disable checking if you must, but in practice the collaborator won't be loaded when you're speccing out the new interface anyways (I think someone mentioned this above).

Along the lines of checking both instance and singleton methods, since we know that a collaborator will either represent a class or an instance of the class, we could check both places.

I don't like this either. Class and instance are two separate collaborators, they need to be treated as such.

[mikegehard]

Thanks for clarifying the differences for the class and instance collaborators. I was having a little bit of a hard time getting them straight in my head.

I'm willing to take a shot at getting this started. I think I have enough to get started at least getting it to function similar to the as_null_object. If there are any gotchas, please let me know.

[dchelimsky]

@msgehard you're welcome to get started on this work so we have something to work with, but just to be clear about this: at this point I am not 100% on board w/ merging this into rspec-mocks. I'm not against it either - just need some time to think about it and I'm pressed for time these days.

I'm planning to get the 2.11 release out next weekend, and I'll try to make some time after that to investigate all this.

[mikegehard]

@dchelimsky Thanks for being clear. I'll get started and submit a pull request where we can continue discussion.

Regardless of wether this makes it in or not, I'll get to learn a little more about rspec-mocks. :-)

Thanks.

[xaviershay]

Closing, re-open if anything new arises.