Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify calls are made in the correct order #748

Closed
tometchy opened this issue Feb 14, 2019 · 7 comments
Closed

Verify calls are made in the correct order #748

tometchy opened this issue Feb 14, 2019 · 7 comments

Comments

@tometchy
Copy link

I use great Verify method often, but sometimes I need to check not only how many times given method was invoked, but also if it was in proper order.

Sometimes even with mixing order of different mocks, for example

// This should be called first
_mockA.Verify(a => a.Method(It.Is<string>(s => s == "a1")), Times.Once);

// This should be called second
_mockA.Verify(a => a.Method(It.Is<string>(s => s == "a2")), Times.Once);

// This should be called third
_mockB.Verify(b => b.Method(It.Is<string>(s => s == "b1")), Times.Once);

// This should be called 4'th
_mockA.Verify(a => a.Method(It.Is<string>(s => s == "a3")), Times.Once);

Currently I use work around basing on callbacks (like posted in stack overflow https://stackoverflow.com/questions/10602264/using-moq-to-verify-calls-are-made-in-the-correct-order)

int callOrder = 0;
writerMock.Setup(x => x.Write(expectedType)).Callback(() => Assert.That(callOrder++, Is.EqualTo(0)));
writerMock.Setup(x => x.Write(expectedId)).Callback(() => Assert.That(callOrder++, Is.EqualTo(1)));
writerMock.Setup(x => x.Write(expectedSender)).Callback(() => Assert.That(callOrder++, Is.EqualTo(2)));

But it has big disadvantage, that I need to prepare assertions before acting, which looks strange. The best I can do is to wrap it in local functions which make it look a bit better, but it's still inconvenient

public void Test()
{
    // Arrange code

    void Act()
    {
        // Act code
    }

    void SetAssertions()
    {
        // Assertion methods
    }

    SetAssertions();
    Act();
}
@stakx

This comment has been minimized.

@stakx
Copy link
Contributor

stakx commented Feb 14, 2019

Hmm, on second thought, please disregard the above post. You said that you don't want to Setup the calls, you want to consider call order right with your Verify calls.

Another little-known Moq feature you could use is argument capturing (e.g. Capture.In):

var args = new List<string>();
mock.Setup(m => m.Method(Capture.In(args)));
// ...
Assert.SequenceEqual(new[] { "a1", "a2", "b1", "a3" }, args);

Note that a call to mock.Verify to ensure Method was called exactly four times is redundant, since you're already checking whether args contains four elements (which will only be the case if the method was called the same amount of times).

@tometchy
Copy link
Author

tometchy commented Feb 15, 2019

I've read about MockSequence before, but in my case it works worse than using callbacks.
I didn't know about Capture.In() and it seems to be better solution, but only if you need to check order of one mock invocations, which is not my case.
My case is that I call few methods on my SUT, and SUT in result call two dependencies in exactly specified order. Moreover SUT must call those two dependencies in proper order, even if user called methods on SUT in wrong order (which is our production case).
With callbacks I achieved that, but as I said before, with having assertions set before acting, which worsen readability. Here is example how this tests looks like exactly

[Test]
public void GIVEN_play_invoked_twice_with_first_play_too_early_canceled_EXPECTED_commands_send_in_proper_order()
{
	// Arrange
	var wavPath = F.Create<string>();
	var wavPath2 = F.Create<string>();
	var playId = F.Create<string>();

	void Act()
	{
		// Act
		_sut.PlayWave(wavPath);
		_sut.CancelPlay();
		_sut.OnPlayStarted(playId);
		_sut.OnPlayComplete(playId);

		_sut.PlayWave(wavPath2);
	}

	void SetAssertions()
	{
		// Assert
		SetVerification(sender => sender.PlayWave(AssertPlayParameters(wavPath)), invocationOrder: 0);
		SetVerification(sender => sender.CancelPlay(AssertCancelParameters(playId)), invocationOrder: 1);
		SetVerification(invoker => invoker.OnPlayComplete(), invocationOrder: 2);
		SetVerification(sender => sender.PlayWave(AssertPlayParameters(wavPath2)), invocationOrder: 3);
	}

	SetAssertions();
	Act();
	AssertVerifications();
}

In SetVerification methods I hided using callbacks which I check in AssertVerifications() method

private void AssertVerifications()
{
	Assert.AreEqual(_expectedInvocationsNumber, _invocationsNumber, "Methods were not invoked expected number of times");
	Assert.IsTrue(_invocationOrderIsCorrect, "Wrong methods invocation order");
}

Tests written with those methods look acceptably, but still much worse than tests which doesn't need to check order of invocations, for example to comparision

[Test]
public void GIVEN_play_invoked_twice_but_play_started_and_play_complete_not_came_EXPECTED_only_first_play_sent()
{
	// Arrange
	var wavPath = F.Create<string>();
	var wavPath2 = F.Create<string>();

	// Act
	_sut.PlayWave(wavPath);
	_sut.PlayWave(wavPath2);

	// Assert
	_mockProtocolSender.Verify(sender => sender.PlayWave(AssertPlayParameters(wavPath)), Times.Once);
	_mockProtocolSender.Verify(sender => sender.PlayWave(AssertPlayParameters(wavPath2)), Times.Never);
}

Update: it might be not obvious, but in first shown test, 'invoker' and 'sender' are two different dependencies (SetVerification method has overloads).

@stakx
Copy link
Contributor

stakx commented Feb 16, 2019

Moreover SUT must call those two dependencies in proper order
[...]
'invoker' and 'sender' are two different dependencies

Short of MockSequence, Moq currently has no other built-in API that would allow you to verify proper invocation order across several mocks. If you cannot, or don't want to, use MockSequence, you'll have to work around this using a counter and/or .Callback() (which is what MockSequence and the Moq.Sequences NuGet package I mentioned above do under the hood). But you've already figured that out.

I've read about MockSequence before, but in my case it works worse than using callbacks.

You mentioned briefly that you don't want to Setup your calls (because it "looks strange") but use Verify directly. Are there any other reasons why MockSequence doesn't work in your case?

Finally, what exactly are you requesting here? Is this issue simply a question about available solutions given the current APIs, or are you requesting a new feature? (If the latter, what should that hypothetical feature look like to be useful to you?)

@tometchy
Copy link
Author

You mentioned briefly that you don't want to Setup your calls (because it "looks strange") but use Verify directly. Are there any other reasons why MockSequence doesn't work in your case?

I thought that MockSequence can be applied only across one mock, if it can combine two mocks then it can be used in my case, but as in solution which I've shown, with having assertions set in the beginning of test.

I thought that extending current .Verify functionality will be nice. Something, to be able to write tests like in GIVEN_play_invoked_twice_but_play_started_and_play_complete_not_came_EXPECTED_only_first_play_sent example shown before, but with having something added to Verify, to know that invocation order is checked also.

For example:

var seq = new MocksSequence();
_mockProtocolSender.Verify(sender => sender.PlayWave(AssertPlayParameters(wavPath)), seq, invocationOrder: 0);
_mockInvoker.Verify(sender => invoker.OnPlayComplete(), seq, invocationOrder: 1);

Or something fluent:

var seq = new MocksSequence();
_mockProtocolSender.Verify(sender => sender.PlayWave(AssertPlayParameters(wavPath))).CallOrder(seq, 0);
_mockInvoker.Verify(sender => invoker.OnPlayComplete()).CallOrder(seq, 1);

Or something like this:

new MocksSequence()
{
    () => _mockProtocolSender.Verify(sender => sender.PlayWave(AssertPlayParameters(wavPath))),
    () => _mockInvoker.Verify(sender => invoker.OnPlayComplete())
}.VerifyInvocationOrder();

Or some static way:

Mock.InSequence(() => _mockProtocolSender.Verify(sender => sender.PlayWave(AssertPlayParameters(wavPath))));
Mock.InSequence(() => _mockInvoker.Verify(sender => invoker.OnPlayComplete());

So it's feature request :)

@stakx
Copy link
Contributor

stakx commented Feb 16, 2019

I thought that MockSequence can be applied only across one mock, if it can combine two mocks then it can be used in my case

Yes, MockSequence does work with several mocks. For example:

public interface IFoo
{
    void Do();
}

var seq = new MockSequence();
var a = new Mock<IFoo>(MockBehavior.Strict);
var b = new Mock<IFoo>(MockBehavior.Strict);
a.InSequence(seq).Setup(m => m.Do())...;
b.InSequence(seq).Setup(m => m.Do())...;
a.Object.Do();
b.Object.Do();

This example uses MockBehavior.Strict to disallow superfluous calls.

What this won't do—and that's one example of MockSequence 's limits—is to check whether all calls in the sequence have been made. You'll still need to .Verify() that with all participating mocks a and b:

a.Verify(m => m.Do(), Times.Once);
b.Verify(m => m.Do(), Times.Once);

So it's feature request :)

The fundamental problem with all of your examples is that there is no global fully ordered record of all mock invocations; in fact, each mock only keeps track of its own invocations, so we have no record of whether a call to a.Do() happened before or after b.Do(). So by the time you get to .Verify(), there's no way to reconstruct the precise invocation order across all mocks.

So at the very least, you'd have to have some way of telling each mock a and b that they should record their invocations in a shared location (say, a MockSequence)... before the invocations happen, i.e. in the setup stage.

The most minimal, but still feasible API when we want to focus on Verify without blowing up the Setup stage might look like this:

// Arrange:
var a = new Mock<IFoo>();
var b = new Mock<IFoo>();
var seq = MockSequence.With(a, b); // sets up `a` and `b` such that they report all calls to `seq`

// Act:
a.Object.Do();
b.Object.Do();

// Assert:
// verifications on `seq` must happen in the same order as the recorded calls:
seq.Verify(() => a.Do(), Times.Once);
seq.Verify(() => b.Do(), Times.Once);
seq.VerifyNoOtherCalls();

This is off the top of my head, without looking at #21 and #75. I am sure there's a ton of use cases I'm forgetting about—e.g. cyclic sequences—for which the above example would be a bad fit.

@tometchy
Copy link
Author

The fundamental problem with all of your examples is that there is no global fully ordered record of all mock invocations; in fact, each mock only keeps track of its own invocations, so we have no record of whether a call to a.Do() happened before or after b.Do(). So by the time you get to .Verify(), there's no way to reconstruct the precise invocation order across all mocks.

I'm aware of it, every example I've given has some 'higher' state.
Btw. thanks for so fast and detailed responses, I'm under really good impression :)

@stakx stakx closed this as completed Feb 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants