This text relates to my earlier proposal of mock.Count method: https://github.com/Moq/moq4/pull/17#issuecomment-6538764
I publish this text here just not to spam the issue tracker with many pages of loose thoughts.
I think that maybe I expressed myself not too clear. I was not saying that Reset is bad/useless/etc in a general manner. I meant that:
- Reset() -- resets whole log - is easy to use
- Reset(filter) -- removes selected entries - has its pros, but despite being similar to Verify, it is much harder (than Verify) to use properly in any non-trivial cases, and in some case, may even be useless while cumulative Verifys would work
- .Count(filter) -- is not as "clean" as Reset(filter) due to needed local variables, but does not have the drawbacks that make Reset(filter) possibly hard to use
If you wonder on drawbacks: it is the state mutation, performed with "greedy" filter.
Facts:
- Verify and Count works intuitively, before they doesn't change anything, and just check or summarize the current state.
- Reset on the other hand modifies the current observation state.
- Filters are built of two types: precise
It.Is<X>( x => spec-of-x ), and range-wise:It.IsAny<X>() - IIRC, even the precise filter can be made range'd, for example with spec:
val >= 15
Imagine a case:
interface IManager
{
void Register(int priority, int itemType);
}
class Tested
{
...... ctor, IManager mgr variable, ...
public void DoWhizz(..params, params..)
{
...blah...
mgr.Register( 0, 20 );
...blah...
mgr.Register( 0, 20 );
...blah...
mgr.Register( 5, 0 );
...blah...
mgr.Register( 5, 20 );
...blah...
mgr.Register( 5, 20 );
...blah...
}
}
void TheTest()
{
Mock<IManager> mock = .....;
var obj = new Tested(mock);
obj.DoWhizz(.....);
// // // mock.Verify(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) ), Times.Exact( 4 ) ); // A, would pass
mock.Verify(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() ) , Times.AtLeast( 3 ) ); // A, passes
mock.Reset(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) )); // B
obj.DoWhizz(.....);
mock.Verify(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) ), Times.Exact( 4 ) ); // C, passes
mock.Verify(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() ) , Times.AtLeast( 6 ) ); // D, FAILS
}This test was written with some black-boxy assumptions:
- during all DoWhizz calls, it should Register at least 3 items with high priority
- during the very first call, we do not care about the item types, as it may for example, reregister some historical things
- during later calls, we want exactly 4 new items of "higher type" to be processed
Line tagged as (A) is obvious. At this point of time the val>15 is ignored, but if I knew that there is no history/etc, I could use it and it would pass.
In line (C) for some reason, I wanted to not accumulate expectations and write 8, for example, for test mainterance reasons.
So, in line (B), I resetted the counter for the calls in question. Maybe I was new to that, or maybe I did not think enough about it, or maybe just my actual case was a bit more complex, the issue nested deeper and harder to trace - whatever. I used the same filter I will use later in C, and also it is all the same filter I'd use in (A) if I could/knew - it "felt obvious and natural".
The effect is that the under-specified reset filter in line B has implicitely mutated counters not only for the first, but also for the second issue that this test checked. I resetted counts for second param, and the count for first param has implicitly changed, and at point (D) it will be now 5, not 6. This is obvious when you look at implementation of the Tested class, and if you know exactly how the invocation log was 'recorded'. Elsewise, this is not so obvious.
To perform proper reset in this case, the reset would have to look like:
mock.Reset(o => o.Bar( It.Is<int>(val => val <= 3), It.Is<int>(val => val > 15) ));and this starts to present the possible complexity of having to manually slice the parameter domain into disjoint regions to be droped. With integers and 1 or 2 params this is easy, but later it will start to be a tough piece to mantain.
Assuming you dont want to analyze and slice the domain, you may try just to "intuitively" reset for the other paremeter too:
mock.Reset(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) ));
mock.Reset(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() ));Right? Not! First note: what if there were more parameters? Add more resets? Second note: Actually, with each such line, you would introduce more uncertainity, because the second line has its second param open. Such two lines aggregate to:
mock.Reset(o => o.Bar( It.IsAny<int>(), It.IsAny<int>() ));so, actually, they are unfiltered reset that clears everything related to that method. If it has helped in our case, then we did not need the filter at all. If it did not help - then the reset must be specified properly by slicing the domain and praying that we have few, separable parameters:
mock.Reset(o => o.Bar( It.Is<int>(val => val <= 3), It.Is<int>(val => val > 15) )); /// <--- the only safe and proper waySo, while it is useful feature, in my view, in most nontrivial cases, some unpleasant cases show up:
- case1: those not fluent in the topic, will "intuitively" expect that line (D) will have the count of 6 - because they have reset/filtered the OTHER parameter
- case2: those more experienced, will know that clash can happen, and will manage to avoid it, but they will be unable to guess what count they should expect at (D), because the DoWhizz's will be a black box
- case3: the parameters will be too entangled, or clash will be deep enough to make the parameter domain too hard to slice properly, thus making it impossible to determine the count at (D) after the partial reset
As I already said, there's an 'oops' that with Verify & Times, the writer has to specify the absolute bounds, and is unable to express relative bounds. The addition of Reset did not help - it solved one assertion by resetting it to zero, but has damaged the other one by making the absolute value hard to determine -- because there is no way to learn the number of invocations other than asserting it Verify & Times and manually reading the exception text.
If Count were added, the case is trivial to fix:
void TheTest()
{
Mock<IFoo> mock = .....;
var obj = new Tested(mock);
obj.DoWhizz(.....);
mock.Verify(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() ) , Times.AtLeast( 3 ) );
mock.Reset(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) )); // <--- damages some counts
int cnt = mock.Count(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() )); // just CHECK them
obj.DoWhizz(.....);
mock.Verify(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) ), Times.Exact( 4 ) );
mock.Verify(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() ) , Times.AtLeast( cnt+3 ) ); // and USE them
}Instead of analyzing what and how to reset and writing several resets to properly clear the parameter domain chunks, I just read the current value and use it in assertion. In my view, it is as short, as easy to comprehend and also as readable as it can be.
The same test with Count only is in my point of view more straightforward, because you dont have to think what hidden sideeffects could the Reset have:
void TheTest()
{
Mock<IFoo> mock = .....;
var obj = new Tested(mock);
obj.DoWhizz(.....);
mock.Verify(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() ) , Times.AtLeast( 3 ) );
int cnt1 = mock.Count(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) ));
int cnt2 = mock.Count(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() ));
obj.DoWhizz(.....);
mock.Verify(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) ), Times.Exact( cnt1 + 4 ) );
mock.Verify(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() ) , Times.AtLeast( cnt2 + 3 ) );
}I hope I managed to show you that:
- Multiple subsequent calls to Count are orthogonal, while calls to Reset are not
- addition of Count overlaps with Reset only partially
- addition of Count helps to deal with some problems of Reset
- Count does not have the problems of reset
- compared to Reset, the Count forces you to add similar numer of "extra lines", usually less (no params slicing)
I wrote a tome, so I'll add a few words more :) The last example shows that Count is used like Verify with some sort of initial checkpoints. It is a little work to write cnt+4, and it may look ugly for some, so I also suggested Times.Add() method for some sugar.
Actually, what I'd like to see is:
void TheTest()
{
Mock<IFoo> mock = .....;
var obj = new Tested(mock);
obj.DoWhizz(.....);
// var mark1 = mock.Mark(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) ) ); // either that
var mark1 = mock.Verify(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) ), Times.AcceptAny() ); // or that
var mark2 = mock.Verify(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() ), Times.AtLeast( 3 ) );
obj.DoWhizz(.....);
mark1.VerifyNext( Times.Exact( 4 ) ); // no more filter copying just to verify the same thing
mark2.VerifyNext( Times.AtLeast( 3 ) );
mark2.Verify( Times.AtMost( 3 ) ); // why not have a Verify and VerifyNext here?
// below - contrived features :)
Assert.True( mark2.Count % 3 == 0 ); // Count returns current last count stored by marker, if the Times is not enough
mark2.Count = 15; // marks are meant to be simple. You can do that if you know better
int cnt = mock.Count( mark1.Observed ); // you can always check the most recent without bumping the markers
var markX = mock.Verify( mark1.Observed, Times.AtMost( 5 ) ); // marker remebers the filter? coooool
}Currently, verify now returns void. This is a waste. This is a great handy way to provide simple checkpointing that could increase readability or offer new sly features. The API change is minimal: Verify works as always, throws when Times are not satisfied. But when succeeds, returns a marker with some information on the last state of the observation.
However, you'd get a marker only when Verifying something. How about if you just want to have a new marker without checking anything? It would need a new factory method, or can be solved just by adding a pass-all option to times solves it.
The Mark/Checkpoint object may be a simple struct that just remembers the Lambda and the Count. The VerifyNext would update the internal counter, so several VerifyNext would behave intuitively. The Count property is only used as a base offset for "next checks", it does not have anything to do with actual invocation history log - it just offsets the Times during VerifyNext. The Observed property (or Filter, or Condition, name is not so important), carries the original expression provided to Verify. The whole Mark is really a Mark
With such thingies, the example from earlier parts would boil down to:
void TheTest()
{
Mock<IFoo> mock = .....;
var obj = new Tested(mock);
obj.DoWhizz(.....);
var mark1 = mock.Verify(o => o.Bar( It.IsAny<int>(), It.Is<int>(val => val > 15) ), Times.Any() );
var mark2 = mock.Verify(o => o.Bar( It.Is<int>(val => val > 3), It.IsAny<int>() ), Times.AtLeast( 3 ) );
obj.DoWhizz(.....);
mark1.VerifyNext( Times.Exact( 4 ) );
mark2.VerifyNext( Times.AtLeast( 3 ) );
}which in my opinion is far superior to addition of .Reset and .Count
There is no rush, and I am aware that most of the people does not do any "incrementality" in their test, they probably just multiplicate the test cases five times and test things separately. I do not negate this approach: sometimes this is good and desired. However, when you have nontrivial setups to perform, you actually want to either reuse as much as possible (even the assertion builders), or to check as much as possible at the cutpoints that you just meticuously crafted.
Addition of .Count is ready and is an instant. Change is trivial and provides similar features to 'marks', just with less syntax sugar. By throwing the Reset in, we add more sugar, but Count is still needed.
When I get another few days off from work, but if I will try to write it, and probably post it as separate proposal.
