On Feb 12, 2:17 pm, clay s <[email protected]> wrote: > Yan, > > The "better whine" is actually a very compelling argument, *in principle*. > But I don't see how it can actually work. E.g. > > * it 'works' do* > * f = double* > * f.should_receive(:bar) { 42 }* > * result = 1* > * # Some dumb engineer commented out the line where we compute the value! > * > * # result += f.bar* > * expect(result).to eq 43* > * end* > > This fails because someone commented out the part where we add f.bar to > result. But the failure doesn't tell us that the code failed to call f.bar. > It just says: > > *expected: 43* > * got: 1* > * > * > *(compared using ==)*
You've succeeded in showing us a contrived example where a stub would make more sense, but that doesn't mean that `should_receive.and_return` can never work. In your example, it doesn't ever fail with a mock expectation error because you have a non- mock expectation that fails first. But consider a spec where the assertion you want to make is that a particular message is received by a particular collaborator object (Yan's example above is an example of this). If the assertion you really want to make is that a particular message is received (and not about the value returned of the method called by your spec), then using a mock expectation is the right tool. If the code-under-test depends upon the mocked method returning a non-nil value then you will have to use `and_return` to get things to pass. I think there's a useful bit of design feedback here (e.g. the use of `should_receive.and_return` suggests you've combined a command and query into one method, and may want to split it), but the fact remains that, contrary to your assertion, using `should_receive.and_return` not only works, but is useful in this case (just as it was in the caching example I gave before). Here's another example...consider a case like: collaborator = double double.stub(:fuzzy?).and_return(true) subject = Algorithm.new(collaborator) expect(subject.result).to eq(47) In your example, the relationship between the stubbed response (42) and the final result (43) was obvious. Here it's very non-obvious: the collaborator has a boolean predicate that plays into the final result somehow, but it's not clear from reading the test how it plays in. One can imagine a future refactoring to `Algorithm` such that it no longer uses `collaborator.fuzzy?`, but, for whatever reason, continues to pass. The use of the stub rather than `should_receive` here causes the test to create confusion, as it suggests that `fuzzy?` is used when, in fact, it may not be. As a pragmatic decision, I think it would be OK to use `should_receive` here instead, as that would cause the example to fail as soon as `fuzzy?` stopped being called. > So the API I'd like to see is: > > - Mocks cannot return values. > - If you want to assert that something is cached (i.e. it shouldn't be > called more than once) then then do something like > double.stub(:foo).with(42)*.cache* I'm not convinced your idea of the `cache` API adds any benefit or clarity over what we already have. > Not particularly optimistic that this change will be implemented in my > lifetime. :) Generally speaking, I'm not in the habit of expanding effort to add code and complexity to make RSpec's APIs less flexible. It would be more code, not less, to disable `and_return` from being available for `should_receive`. I've got more important things to work on. I imagine you could easily create an external gem that provides this functionality. That would be pretty trivial, and honestly, it probably would have taken less time then this whole discussion. -- You received this message because you are subscribed to the Google Groups "rspec" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
