Provide way to assert completed calls without using Repeated class

[blairconrad]

As mentioned by @jholovacs in #808, the Repeated in A.CallTo(() => f.Boo()).MustHaveHappened(Repeated.Exactly.Once) could be construed as meaning "Boo must have happened, and then it must have been repeated, for a total of two calls". After much discussion, we've resolved to provide alternative methods and eventually deprecate and remove Repeated.
As noted in that issue, we'll introduce the following API:

MustHaveHappened()    // as today
MustNotHaveHappened() // as today

MustHaveHappenedOnceExactly()
MustHaveHappenedOnceOrMore()
MustHaveHappenedOnceOrLess()

MustHaveHappenedTwiceExactly()
MustHaveHappenedTwiceOrMore()
MustHaveHappenedTwiceOrLess()

MustHaveHappened(3, Times.Exactly)
MustHaveHappened(3, Times.OrMore)
MustHaveHappened(3, Times.OrLess)

MustHaveHappenedANumberOfTimesMatching(n => n %2 == 0) // the argument is an Expression<Func<int, bool>>

Times is a new class.

This satisfies the following goals:

1. Eliminates the confusion over the exact semantics of Repeated
2. Retains MustHaveHappened() and MustNotHaveHappened(), which are the most popular methods in use today
3. Improves discoverability, even over the existing Repeated mechanism
4. Provides a consistent, natural word ordering (to contrast with Repeated.Times(3))
5. Does not require an extension method on int
6. Avoids forgettable terminal operators (e.g. MustHaveHappened(3).Times, where the Times could be forgotten)
7. Allows for a soft transition away from the current methods; the Repeated variants can live for a major release or two and be deprecated in 5.0.0 and removed in 6.0.0.

[thomaslevesque]

MustHaveHappenedOnceExactly()
MustHaveHappenedOnceOrMore()
MustHaveHappenedOnceOrLess()

MustHaveHappenedTwiceExactly()
MustHaveHappenedTwiceOrMore()
MustHaveHappenedTwiceOrLess()

I wonder if the names could be improved. I guess OnceOrMore or TwiceOrLess are okay, but OnceExactly is not very natural... ExactlyOnce is more idiomatic (correct me if I'm wrong).
If we make this change, for consistency we should probably not mix ExactlyOnce with OnceOrMore/OnceOrLess, so we could use AtLeastOnce()/AtMostOnce() instead.

So we would have this instead:

MustHaveHappenedExactlyOnce()
MustHaveHappenedAtLeastOnce()
MustHaveHappenedOrMostOnce()

MustHaveHappenedExactlyTwice()
MustHaveHappenedAtLeastTwice()
MustHaveHappenedAtMostTwice()

What do you think?

[thomaslevesque]

Ah, I just realized the names you had suggested are consistent with MustHaveHappened(3, Times.Exactly)... So maybe it's better to stick with that.

[blairconrad]

2 minutes before I push my "final" changes you come up with this? You're killing me, Levesque.

Idiom is in the ear of the beholder, but I'd agree that "once exactly" is slightly less natural than "exactly once". On the other hand, I see you just commented with the response I was going to make.

[thomaslevesque]

So... which is more important? Being idiomatic, or being consistent? I'd say consistent is more important (in which case we can keep the names you suggested)

[blairconrad]

I think being consistent is enough of a benefit to compensate for the sightly stilted phrasing. At least names like MustHaveHappenedOnceExactly lead the writer (assuming they're using some form of smart completion) to "Once" right away, which is probably the key concept. Then they see the 3 choices to complete the method name and pick "Exactly".

[blairconrad]

This change has been released as part of FakeItEasy 4.4.0.

Thanks for your help with this, @jholovacs. Look for your name in the release notes! 🏆

[Mertsch]

@blairconrad I would like to understand why you aren't deprecating the APIs right now.
I am thinking of guiding people to the new API before it gets deprecated in 5.0.0

[blairconrad]

@Mertsch, by all means guide people to the new APIs. I would.
And we have an update to the Analyzer in the works that will also guide them as soon as it's done.

The main reason we did not deprecate the APIs yet is that that can be considered a breaking change. It's common in the OSS framework ecosystem^{citation needed} to deprecate an API in a major release and remove it in the following major release. Accordingly, we should deprecate in 5.0.0 and remove in 6.0.0. We don't yet have any timing around a 5.0.0 release, but it is something we could investigate.

[Mertsch]

@blairconrad OK, if that is common practice, I would have expected to mark

FakeItEasy/src/FakeItEasy/Configuration/IAssertConfiguration.cs

Line 25 in 95e9f30

UnorderedCallAssertion MustHaveHappened(Repeated repeatConstraint);

with [Obsolete] in 4.4.0 and remove it with 6.0.0 to give it some more time then 5.0.0
Thanks for the explanation 👍

[blairconrad]

One concern I'd have with deprecating in 4.4.0 is that any consumers of the old API who have TreatWarningsAsErrors will find themselves unable to build when they pick up 4.4.0. This may be a bit of surprise for a minor release. Of course, one could say that they had it coming by turning on that flag…