Don't allow at_least(0)

[dchelimsky]

See #132 for background.

This statement doesn't really test anything:

object.should_receive(:message).at_least(0).times

It will pass if the object receives the message any number of times, or never really receives it. It is really just a stub.

Let's raise an error in 3.0 recommending the user use stub instead, and add a deprecation warning in a 2.x release with the same message.

[lsegal]

As I mentioned in the other thread, I disagree with this change. There's an added benefit to having a configurable N value, namely that you don't need to care what "N" is, as long as N>=0. This allows you to implement both a "stub" like scenario and a straight up mock with the same interface. Consider the value for DRYing up specs that share the same behaviour except the N value, namely:

%w(foo bar baz).each do |name|
  it "should have its 'a' characters replaced with '_'" do
    name.should_receive(:sub).with('a', '_').at_least(name.count('a')).times
    # complicated setup and spec behaviour here, calling some method
    # that replaces 'a's in +name+.
  end
end

Note that bar and baz will receive once, but foo should receive zero times.

Disallowing this syntax means you have to have a special case just for foo, which leads to a more complicated test. I see no logical reason why this behaviour needs to be restricted, it seems like an artificial limitation to me. A mock shouldn't care how many times it is received. Given the functionality benefits illustrated above, there's use in this feature sticking around.

[Confusion]

@lsegal: I don't think that example is convincing. The test does not assert

foo should receive zero times

It asserts 'foo is received zero or more times', which is equivalent to 'I do not care what processing foo results in'. It effectively tests nothing when processing foo (well, except that it doesn't raise an error). Why not use exactly instead of at_least here? In that case, 0 would actually assert something.

@dchelimsky I'm not familiar with the code, but I could imagine at_least not accepting 0 would also result in exactly not accepting 0? I think it is reasonable for exactly to accept 0 and effectively act as should_not?

Edit: on second thought, that parenthetical comment

(well, except that it doesn't raise an error)

is actually interesting. Isn't it reasonable to want to process an edge case that should not result in any observable behavior with the same spec that verifies observable behavior in other cases? In that case the line at_least(0).times indeed doesn't do anything, but the invocation of the method being tested still at least verifies it doesn't fail catastrophically.

[lsegal]

@Confusion you could use exactly(). Certainly if exactly(N).times allows N=0 then that is even more of a reason why the API should be consistent across all methods that take .times. In my example however, I don't care about an exact match, just a minimum amount. For instance, my implementation could attempt to sub() 'a' more times than necessary, but I don't have to necessarily care about such an inefficient implementation detail. But as you said, exactly should be able to work here, too-- and if exactly() does, at_least() should work the same.

[lsegal]

@Confusion by the way, it doesn't quite mean "I don't care what processing foo results in". If you were to write out the spec for foo individually, yes, it would mean that, and it would seem a little odd, but that is not what is being done here. The whole point is that in the context of batch specs, I'm not thinking about each individual matcher call. The overall spec should translate to, "I'm giving you a list of data, and I want sub to be called on each item a certain number of times that is relative to the item itself". It seems weird to limit that kind of flexibility when it has already been supported and can't be achieved any other way (except for explicitly checking N=0 and performing a completely different match, which is more of a workaround than a solution).

I think it comes down to this: yes, this expression can be rewritten in a more accepted form, and yes, this would be weird if it were written out literally as mock.should_receive(:method).at_least(0).times. But that's not what is happening, and, ultimately, RSpec shouldn't care even if it did. Just because there is a more appropriate form, does not mean RSpec should disallow alternate forms that are still logically valid. I should reiterate, at_least(0).times might not be "idiomatic", but we should agree that it is logically valid. For that reason alone, RSpec should allow it.

[dchelimsky]

@lsegal saying should_receive(:sub).at_least(0) times is deceiving because it looks like it's testing something but it's not actually testing anything. This is true whether it's 0 or N = 0 IMO.

I should reiterate, at_least(0).times might not be "idiomatic", but we should agree that it is logically valid.

So is at_least(-1).times to a computer, but it makes no sense to a human who is trying to understand a failure. You argue that we should support N >= 0, but that "explicitly checking N=0 and performing a completely different match ... is more of a workaround than a solution". If we're going to be specific about N >= 0, then we can be specific about N >= 1.

@Confusion at_least, at_most and exactly are all methods, and are free to handle this differently. If we do this, we should probably have the following constraints:

exactly # >= 0
at_least # >= 1
at_most # >= 1

at_least(0) should really be a stub, and at_most(0) should really be exactly(0) or never.

@lsegal in terms of your argument about keeping specs DRY and how this change would force you to special case things, I'd argue that this is a good thing! 0 is a special case for at_least (not so for exactly), and not treating it as such risks false positives.

BTW - I'm not 100% sold on this myself. Nobody has actually complained of being bitten by a false positive stemming from at_least(0), but I have been bitten by a similar issues in other mock frameworks. I'm going to leave this open for discussion. Curious to hear some other opinions, and maybe some new arguments one way or the other.

[bjeanes]

@lsegal in terms of your argument about keeping specs DRY and how this change would force you to special case things, I'd argue that this is a good thing! 0 is a special case for at_least (not so for exactly), and not treating it as such risks false positives.

This. A thousand times, this. If 0 is a special case, it should have a test to draw attention to it. Special cases are important and shouldn't be hidden in code.

DRY, as a principle, is about logic more than code. If your DRYing out your tests, you're probably doing something wrong. Furthermore DRYing can introduce coupling between concepts and should only be done for logic that is truly "logic-ing" the same (business) process. If two separate processes happen to (perhaps temporarily) have the same or similar logic, DRYing is still questionable.

FWIW, I'm totally in favor of this issue's proposition.

[yellow5]

I do not think that at_least(0) makes sense. I agree that it is basically a stub, and it does not set any expectation on usage of the appropriate code.

Borrowing the example of @lsegal

  it "should have its 'a' characters replaced with '_'" do
    # May pass, but it really just stubs the call.
    # The code may indeed call 'foo'.sub('a', '_') and the test would pass.
    'foo'.should_receive(:sub).with('a', '_').at_least(0).times

    # Much clearer what is being tested.
    'foo'.should_receive(:sub).with('a', '_').never
  end

The biggest risk of at_least(0) has already been mentioned: false positives. I think the proposed change makes sense and that it promotes better specs.

[lsegal]

There seems to be a dangerous attempt to conflate the stylistic opinion that "this is not proper style" with the logical stance that "this does not make sense" in order to win what is effectively a stylistic argument. Again, I have to reiterate that saying "at_least(0).times does not make sense", well, doesn't make sense. If this decision gets made on a stylistic basis, please be clear about that. Calling this "illogical" because "you should be using method Y" is using science to win an aesthetic argument.

So is at_least(-1).times to a computer, but it makes no sense to a human who is trying to understand a failure.

This is not a fair statement. We're not discussing what makes logical sense to a computer (and it doesn't, by the way), we're discussing what makes logical sense to a human. The statement at_least(-1).times makes no logical sense to a human, because a message cannot be received -1 times. On the other hand, 0 is a number of times with which a message can be received by an object.

This is a simple set of questions that should clarify the boundaries of .times:

1. Can an object receive a message -1 times? Y/N
2. Can an object receive a message 0 times? Y/N
3. Can an object receive a message 1 times? Y/N
4. Can an object receive a message 2 times? Y/N

We all answer "No" to Q1 and "Yes" to Q3 & Q4, obviously. However, if you answer "No" to question 2, you are contradicting RSpec's own functionality. As pointed out, an object can receive a message 0 times, via .never. If you answer "Yes, but!" to Q2, then you admit this is a stylistic issue, not a logical one. The fact that an object in a runtime environment of any programming language can receive a message zero times means that an object can also receive a message at_least zero times. This is basic inductive logic.

Furthermore, I call on RSpec's own method library to show that at_least(0).times is valid. rspec-mocks documentation shows should_receive(:foo).any_number_of_times. How is any_number_of_times logically different than at_least(0).times? If any_number_of_times is allowed, why would the exact same behaviour written in another way be disallowed? This is a stylistic issue, not a logical one.

I question the new constraints on exactly, at_least and at_most. It makes less sense to impose arbitrary restrictions on logic in a logic testing library. If exactly(0).times works, we admit that a message can be received zero times. Again, inductive logic shows that an object can therefore receive a message "zero or more times", or, "at least zero times". Changing this for everything but exactly() creates an inconsistent API. Remember that the receive count methods are just qualifiers to how we bound .times. In other words, at_least means >=, exactly means =, and at_most means <=, for any valid value of N. Note the important part: we're not making any restrictions on N here, just the comparison qualifier. Therefore, if it is valid to say N = 0, then it must be valid to say N >= 0. We didn't change N, we just changed the comparison.

In this context, if the API allows for a bounding of messages such that {x ∈ Z | x ≥ 0 ^ N = x} but does not allow for {x ∈ Z | x ≥ 0 ^ N ≥ x}, then the API is fundamentally broken when x = 0. It is also stylistically wrong to have inconsistent APIs. The API shouldn't care which comparator is used, the bounding of times is simply: {x ∈ Z | x ≥ 0 ^ N OPERATOR x}. We shouldn't have different rules for different qualifiers. That's just messy and unnecessary. FWIW, any_number_of_times is explicitly the above comparison where the comparator is >= and x = 0. This is equivalent to at_least(0).times. It also happens to read the same in English, to me, but I suppose that's not true for everyone else here?

Finally, since this is a debate on style, my opinion is this: RSpec should stay out of making stylistic decisions for its users. We are all consenting adults here, and we can identify when we break recommended idiomatic patterns in order to obtain certain functionality. Ruby would not exist were it not for this possibility. It's one thing to advocate for a certain style (in documentation, in best practice guides), it's another thing to restrict functionality because of stylistic expectations. If something is logically valid (as proven above), and, not only that, but also has an accepted equivalent method (any_number_of_times == at_least(0).times), then RSpec should not restrict one but not the other; this is inconsistent and incorrect. I don't mind advocation, or even people on soap boxes telling me I'm doing it wrong, but please don't come into my codebase and yank out my code. Ruby is not TOOWTDI.

[dchelimsky]

@lsegal this is not about style. "should_receive" means "if this doesn't happen there should be a failure". x.should_receive(y).at_least(0).times will never, ever, ever fail.

x.should_receive(y).exactly(0).times however, means something and can fail.

Agreed that any_number_of_times is just like at_least(0).times and should be deprecated along with this change if we do this.

Your points about consistency suggest we should make all three of exactly, at_least, and at_most require N >= 1 and remove should_not_receive, in which case the only way to say it should not be received at all would be to use object.should_receive(:message).never. That would be fine with me as it would simplify the overall API.

[lsegal]

@lsegal this is not about style. "should_receive" means "if this doesn't happen there should be a failure". x.should_receive(y).at_least(0).times will never, ever, ever fail.

But that is style. Yes, it's a tautology, but tautologies are not logically incorrect, they are only stylistically superfluous. RSpec doesn't disallow users from writing tautologies anywhere else that I know of. You can happily write things like:

:red.should == :red

Except that in the case of should_receive, it also has a functional side benefit of setting up a stub for me at the same time, but without using a separate syntax. In other words, I can keep my tests consistent using the same syntax, rather than having to rewrite tests just because N=0 in some cases. Using the same syntax across my tests leads to more maintainable code. If I happen to increase N for certain tests later on, I don't need to change my ".stub" to ".should_receive". That's way simpler for me to manage. Yes, I know I want a stub in this specific case, but I don't really care about the distinction in this case. And RSpec shouldn't care that I don't care, because functionally they both do the same thing. Neither will ever fail, and both will setup a stub. If RSpec cares about the distinction, even though they behave exactly the same in practice, I consider that style.

What I've been meaning to ask is this: is there a technical reason why this syntax needs to go? Again, without a technical reason, this seems purely stylistic.

[dchelimsky]

We're talking about a DSL with human readable words in it that mean things to humans who read and write them. stub and should_receive have different semantics: stub doesn't care whether a message is received, should_receive does. Neither should_receive(x).at_least(0).times nor any_number_of_times care whether a message is received, thereby breaking with those semantics. That they are functionally the same as stubs is an oversight related to sharing code and not considering the semantics carefully; we simply got it wrong.

I've had a similar debate with @jimweirich over should_receive in flexmock, which, by design, doesn't fail the following scenario:

describe Thing do
  it "expects something" do
    thing = Thing.new
    thing.should_receive :foo
  end
end

The reason is that in flexmock, should_receive means 0 or more times unless you explicitly constrain it. Jim's position is that the design of flexmock is such that a mock starts off flexible and becomes more constrained as you add explicit constraints like should_receive(x).once. I have no argument with that intent, but should_receive does not convey that intent and I've talked to several people over the years who've been bitten by this. Changing the name to something like 'should_allow' or some such would make all the difference.

In our case, we have two different words: stub and should_receive, and I do not consider them interchangeable.

[lsegal]

We're talking about a DSL with human readable words in it that mean things to humans who read and write them.

This is exactly why I say style. I completely understand the DSL, but when I (a human) read "object should receive :message at least zero times", I (a human) understand that this is a stub. RSpec still might call this a mock internally, but the distinction is an irrelevant technicality by that point. RSpec shouldn't police how humans interpret the "English". Humans should perform that policing through their own conventions. This is where we might disagree, of course.

In our case, we have two different words: stub and should_receive, and I do not consider them interchangeable.

The concepts of a "stub" and a "mock" should be defined by their properties, not how they are declared. A certain "thing" is a mock if (in RSpec's context of should_receive) it requires a message passed to it. In other words, it's not a mock because you said obj.should_receive, it's a mock because it has to receive a message at least one time. Similarly, a stub is not a stub because you said stub, it's a stub because of the properties-- namely, it does not need to receive the message; this semantic can be expressed in a multitude of ways right now, and RSpec shouldn't care how it is expressed. Again, caring how it is expressed is a job for project conventions or even a best practices guide, not the library implementation itself. Not unless there's some important technical reason, anyway (you haven't answered that question, BTW).

I should point out, by the way, that the distinction between Jim's flexmock and RSpec don't seem as significant as you make them. Frankly, the only difference is the initial constraint applied to should_receive. In RSpec, the default constraint is .once unless otherwise overridden-- in flexmock, the default constraint is [0,inf[. This is just an issue of "defaults", not an issue of what "should_receive" means. In both cases, should_receive means the same thing-- the only difference is what the initial bounds are. FWIW, I like RSpec's default bounds more (a mock by default), but that doesn't mean I shouldn't be able to change those constraints to "0 or more times" (and make it a stub) if I want to. This follows my previous paragraph, namely, that defining "should_receive" to mean "this is a mock" is wrong, IMO. "should_receive" just means an object should receive some message (by default one time)-- it becomes a "mock" or "stub" depending on the properties-- again, the default properties in RSpec make it a mock unless overridden, and this is acceptable, but should allow reconfiguration to a stub. Otherwise you are overloading the "English" of the DSL "should_receive" to mean something other than the English text "should receive", because an English statement "object should receive X" does not imply "at least once".

[dchelimsky]

but when I (a human) read "object should receive :message at least zero times", I (a human) understand that this is a stub

• this requires additional (and I'd argue unnecessary) thought
• your original examples don't explicitly say 0, they say N. If that results in a false positive, the reader has to first discover that the value was 0, then contemplate that this is really a stub.

The concepts of a "stub" and a "mock" should be defined by their properties, not how they are declared.

I hear you, but don't agree. If I write a method named check_please that delivers a steak to the table, that's going to be pretty damned confusing, regardless of the properties of the object returned by check_please. Admittedly a stretch of a metaphor, but I think it makes my point. The name should suggest the properties of the object returned, otherwise names mean nothing and can't be trusted.

because an English statement "object should receive X" does not imply "at least once".

I think that, absent any further clarification, it implies exactly once, which is why that's the default.

[bjeanes]

because an English statement "object should receive X" does not imply "at least once".

That's just not correct. Furthermore, in RSpec, 'should' has a very clear and distinct meaning. If something 'should' happen, it is a failure if it doesn't. (1 + 2).should == 3 doesn't mean "I'll allow it to be 3" it means "if it isn't 3, something is wrong".

The same should apply here: if an object should receive a message and it does not, that is a failure.

[justinko]

Considering the semantics of a mock expectation, if it does not have a pass & failure case, it is not a mock expectation. This is the reason why we have stubs.

x.should_receive(y).at_least(0).times, any_number_of_times, etc. do not set failure cases, therefore are not mock expectations. By allowing them to go on, we are blurring the line between a mock expectation and a stub - there is no excuse for this.

[dchelimsky]

is there a technical reason why this syntax needs to go?

No, there is no additional technical motivation for this. at_least(0) works now, and the only reason to change that is if we decide it should be changed.

[Confusion]

A point that I feel has not been addressed in the discussion above, is what I noted in the 'edit' in my first response: that it seems convenient and practical to have something like

%w[foo bar baz].each do |name|
  it "should do something" do
    name.should_receive(:method).at_least(method_determining_count(name).times
  end
end

work properly, even if one of the names results in a count of 0.

This discussion is now primarily about what the semantics of various constructs should ideally be, but I think we are losing sight of the practical implications of the proposed change. Perhaps this use of at_least should flag a warning in a -w mode, but not be prevented otherwise?

[lsegal]

because an English statement "object should receive X" does not imply "at least once".

@dchelimsky @bjeanes, sorry, upon re-reading I realized I had missed an important word, let me re-phrase:

An English statement "object should receive X" does not only imply "at least once".

The important part is emphasized. I understand that "I should receive a letter from the bank every month" should mean "once" (or at least once). The fact is, however, I can also say, "I should receive any number of letters from the bank". This statement is unambiguous, and you would understand me if I said this to you. You would not say, "but, that

Considering the semantics of a mock expectation, if it does not have a pass & failure case, it is not a mock expectation. This is the reason why we have stubs.

@justinko I understand this, but the point is that should_receive should not necessarily imply "mock", and even in 2.x, given the existence of .any_number_of_times, it never has needed to imply this. There is nothing in the term "should receive" that means "mock" to me. Mock comes from the existence is a fail scenario, as you pointed out, which is dependent on how you define the properties on your doubles, not the doubles themselves. Even Martin Fowler points this out in "Mocks aren't Stubs". He dutifully points out that the difference is more subtle than syntax, and doesn't need to be differentiated by syntax.

The problem, I think, is that there seems to be an ideological stance on what "should receive" should mean, and this is keeping us from having a practical discussion on the usefulness of a configurable N value in .times. @Confusion is right, we should be having a practical discussion, not an ideological one.

On the note of practicality, I should point out that just about every mocking library I've researched supports a mock expectation (not a stub) with "zero or more" times.

1. EasyMock: http://www.easymock.org/api/easymock/3.1/org/easymock/IExpectationSetters.html#anyTimes()
2. jMock: http://www.jmock.org/cheat-sheet.html (see Expectations)
3. Python's mock module: http://www.voidspace.org.uk/python/mock/mock.html#mock.Mock.assert_any_call
4. Ruby's mocha has explicit support for 0 cardinality: https://github.com/floehopper/mocha/blob/master/lib/mocha/cardinality.rb - note that mocha doesn't even care about the stub vs mock differentiation in syntax, it's purely defined by the Cardinality property of the double.

With all of these other libraries supporting [0,infinity[ cardinality, why does RSpec need to be different? It seems extremely trivial to support the case of N=0 (it's not even a special case). Ruby's mocha shows just how easy it is in the above rb file.

[dchelimsky]

I don't disagree with your practicality argument: it is easier for you to set things up in loops if at_least supports 0, and it is technically simpler for rspec to continue supporting 0 than to special case it. But I disagree that practicality is the only consideration, or even the most important consideration.

rspec is a testing framework (there, I said it!). A message expectation is a form of a test. A test needs to have a success mode and a failure mode, otherwise it is not only not a test, but it is an impostor deceiving you into thinking it's a test, giving you a false sense of security that your code works as expected. It is a lie.

You've argued that "should_receive" need not mean a message expectation, but we simply disagree. There is a mental model that we all build up about these things, and your mental model seems to be that should_receive and stub return interchangeable things. My mental model marries should_receive to a message expectation (that should pass or fail) and stub to a substitute implementation of a method that doesn't care what happens.

Admittedly, I never noticed the agregious violation of this mental model perpetrated by should_receive(:message).at_least(0).times until this thread came along, but now that you've brought it to my attention, I recognize it for the confusing evil that it is. I was alerted to the any_number_of_times problem just a few days ago, and I believe that if we do stop supporting at_least(0).times, we should also remove any_number_of_times.

As for "that's how everybody else does it", I call your attention to YARD, which strays from previous de facto standards. If we agree that mocks aren't stubs, and we agree that the difference is that mocks are tests and stubs are not, then all four of the frameworks you cite got this wrong too.

[lsegal]

It is a lie.

Lots of tests can be lies. The difference is in this case, the lie also benefits from setting up a stub, so it's not just a lie; it's just a difference in interpreting behaviour. At least, I'm not using it as a lie.

The more interesting question is: how many people are bitten by this lie as opposed to any other tautological logic misstep? My guess is that the number of people who have shot themselves in the foot with at_least(0).times approaches somewhere close to the very N value you'd want to drop support for. Even issue #131 just looks like a simple theoretical thought experiment, not something that actually went wrong in real world code.

Also, even if it is a lie, RSpec doesn't need to yell, "I won't let you do this because you're not testing anything!". It can easily just warn me about this and let me silence said warning if I understand what I am doing.

but now that you've brought it to my attention, I recognize it for the confusing evil that it is.

I'll know better than to make sure my behaviour is within your world view before bringing future bug reports to your attention, then :P I certainly did not expect a regression report to cause the removal of a feature that has been working for 4+ years. Can't say I'm happy about that.

As for "that's how everybody else does it", I call your attention to YARD, which strays from previous de facto standards.

I'm very glad you brought this up. YARD might impose certain defaults on users, but there's a big difference between the way YARD does things and the mentality behind this proposal. In YARD, if we stray from the "default", we provide a working path for users to achieve the same results. In some cases, we support the "defacto standard" even though we don't think it's "correct". In fact, (I'd like to think) we never make decisions based in ideology unless there is a good technical reason, or if we just don't have the resources to implement something. In this case, there is no technical issue, and the feature exists, so there is no resources issue either.

It's also important to realize that YARD is built around the concept of extension, so although you might not consider "write a plugin" to be a valid response, in our case it is. Note, however, that whenever we provide a "write a plugin" answer, there is always a supported public API to make said plugin. If you provided me a public API to make at_least(0).times do what I need to, I would gladly backport this code into my test helpers-- but I'd need to know that it won't break in a future release.

If we agree that mocks aren't stubs, and we agree that the difference is that mocks are tests and stubs are not, then all four of the frameworks you cite got this wrong too.

Stubs are not tests, but stubs are used in tests. There's no reason for RSpec to police the distinction when it already supports both. The distinction is often subtle, and many times not even relevant. The example above is one of those cases. I propose that RSpec treats its users like adults and lets them decide what those distinctions are.

By the way, I have no problem with documentation telling people they will burn in hell for using at_least(0).times in big red bold-face type, or even if you hired 100 mechanical turks to comb through github leaving nasty comments every time they come across such a test, but I still don't see a reason to remove the behaviour outright. As a sidenote, deprecation does not imply removal, especially in the case where there is no maintenance cost. So I'm also fine with deprecating but not removing the feature. Deprecation would be the ultimate way to tell people "don't touch this". We've use this tactic in YARD with no intention of removing several features that have been around for 3+ years now.

[dchelimsky]

At least, I'm not using it as a lie.

Actually, I think you are, but that's admittedly subjective.

The more interesting question is: how many people are bitten by this lie as opposed to any other tautological logic misstep?

Agreed. Probably not many, if any (I haven't heard of this particular problem).

By the way, I have no problem with documentation telling people they will burn in hell for using at_least(0).times in big red bold-face type, or even if you hired 100 mechanical turks to comb through github leaving nasty comments every time they come across such a test, but I still don't see a reason to remove the behaviour outright. As a sidenote, deprecation does not imply removal, especially in the case where there is no maintenance cost. So I'm also fine with deprecating but not removing the feature. Deprecation would be the ultimate way to tell people "don't touch this".

Deprecation and mechanical turks it is!

If you provided me a public API to make at_least(0).times do what I need to, I would gladly backport this code into my test helpers-- but I'd need to know that it won't break in a future release.

Probably won't do this for this, but that is the same approach I try to take for all things rspec these days, so if we ever did actually remove it I'd be OK w/ exposing an extension for it. Also, although we haven't formally declared semver (we don't meet all the formal API requirements yet) we do honor it as much as is feasible (only bug fixes on patch releases, no intentional breaking changes on minor releases), so if we did decide to actually stop supporting this (or anything else) it would come in a major release.

[baob]

Even issue #131 just looks like a simple theoretical thought experiment, not something that actually went wrong in real world code.

No, #131 was a real-world issue. We did some refactoring, then said to ourselves, hold on, that should have broken a spec. On looking at the spec, that was what we found. The coders intent was obviously (from the context which I haven't made public) that this particular method should be invoked once or more. However he coded 'should_receive ... any_number_of_times' and missed the fact that this allowed zero times.

[lsegal]

At least, I'm not using it as a lie.

Actually, I think you are, but that's admittedly subjective.

FWIW, a little extra background on the context behind my use-case: the should_receive's in our spec aren't the actual tests, they are just setup. In other words, we could easily replace all of the shr's with stubs and the test would still have a purpose, so they effectively are being used as stubs here-- it is not a lie. However, we also take advantage of shr's assertions to provide some extra sanity checks in the tests. In other words, we are providing some extra checks to "test the test"; if the tests pass but don't call on h() (the stubbed method), we know something in the test is broken (as opposed to impl). This has actually bit us before, so these shr's have proved useful. In the case where N=0, the sanity check doesn't apply, since there is no possible "insanity", so the noop on the assertion doesn't miss anything.

[bjornblomqvist]

This change would remove an edge case where the api does not assert anything. I think it would be great to make the meaning of should_receive more consistent.

I can't see why writing misleading code would be of any use. I believe its misleading as its general expected that should_receive means that something should be received.

[deepak]

want to test methods being called on rails associations.

class User < ActiveRecord::Base
  has_many :facebook_accounts
end

class FacebookAccount < ActiveRecord::Base
  belongs_to :user
end

in my tests i do not care that user.facebook_accounts is called
But if it is called then, user.facebook_accounts.build(with_some_params) should be called

So the tests should pass if:

1. user.facebook_accounts is not called
2. user.facebook_params.build(uid: 1234) is called

[myronmarston]

@deepak -- I have no idea what you're asking or what the connection is to this issue. If you have a general question about how to test something, github issues isn't the place to do that. Stackoverflow is a better forum for that. Thanks.

[deepak]

@myronmarston i thought, in this case facebook_accounts can be called at_least(0) times
user.facebook_accounts is optional but if called then build needs to be called on the method chain as well
the point being, it seems like a common rails idiom to violate the law of demeter (but maybe only in the code i write or see)

i sent a mail to rspec-users some time back, if that is ok ?

thanks for responding

[myronmarston]

I think you could do something like:

fb_accounts = user.facebook_accounts
user.stub(:facebook_accounts) do
  fb_accounts.should_receive(:build).with(some_params)
  fb_accounts
end

[deepak]

thanks @myronmarston that cleared on how a stub can be used.
Have asked the question on rspec-users at https://groups.google.com/d/topic/rspec/Dc98rP-3IFM/discussion

[kurtisnelson]

Was a decision ever reached on this issue? It looks like the consensus was that at_least(0) and any_number_of_times should raise a deprecation error when used but not be disabled.

[fables-tales]

Closing because of #231 being merged.