MockSequence and VerifyAll #75

Open
henriquemotaesteves opened this Issue Dec 18, 2013 · 15 comments

Comments

Projects
None yet
7 participants
@henriquemotaesteves

When using a "MockSequence" the behavior of the "VerifyAll" method is modified. Is this correct?

public interface IFoo
{
    void Do();
}

/*
 * Throws MockVerificationException
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);

foo.Setup(f => f.Do());
foo.VerifyAll();

/*
 * Does not throw MockVerificationException
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);
var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.Do());
foo.VerifyAll();
@kzu

This comment has been minimized.

Show comment
Hide comment
@kzu

kzu Dec 19, 2013

Member

Good point. What should be the expected behavior?

/kzu from mobile
On Dec 18, 2013 7:33 PM, "Henrique Esteves" notifications@github.com
wrote:

When using a "MockSequence" the behavior of the "VerifyAll" method is
modified. Is this correct?

public interface IFoo{
void Do();}
/* * Throws MockVerificationException /var foo = new Mock(MockBehavior.Strict);
foo.Setup(f => f.Do());foo.VerifyAll();
/
* Does not throw MockVerificationException */var foo = new Mock(MockBehavior.Strict);var seq = new MockSequence();
foo.InSequence(seq).Setup(f => f.Do());foo.VerifyAll();


Reply to this email directly or view it on GitHubhttps://github.com/Moq/moq4/issues/75
.

Member

kzu commented Dec 19, 2013

Good point. What should be the expected behavior?

/kzu from mobile
On Dec 18, 2013 7:33 PM, "Henrique Esteves" notifications@github.com
wrote:

When using a "MockSequence" the behavior of the "VerifyAll" method is
modified. Is this correct?

public interface IFoo{
void Do();}
/* * Throws MockVerificationException /var foo = new Mock(MockBehavior.Strict);
foo.Setup(f => f.Do());foo.VerifyAll();
/
* Does not throw MockVerificationException */var foo = new Mock(MockBehavior.Strict);var seq = new MockSequence();
foo.InSequence(seq).Setup(f => f.Do());foo.VerifyAll();


Reply to this email directly or view it on GitHubhttps://github.com/Moq/moq4/issues/75
.

@kzu

This comment has been minimized.

Show comment
Hide comment
@kzu

kzu Dec 19, 2013

Member

But isn't that what verifying the sequence achieves?

/kzu from mobile
On Dec 19, 2013 10:20 AM, "Henrique Esteves" notifications@github.com
wrote:

In my opinion, regardless if you are using a sequence or not, the behavior
of a strict mock must be the same. If "VerifyAll" is called it should check
if all methods were called.

When using a sequence, the "VerifyAll" method should be more restrictive,
not less, as in the example above. Besides checking all the methods were
called, it should check whether they were called in the correct order.

public interface IFoo{
void DoOne();
void DoTwo();}
/* * Does not throw MockVerificationException /var foo = new Mock(MockBehavior.Strict);
foo.Setup(f => f.DoOne());foo.Setup(f => f.DoTwo());
foo.DoOne();foo.DoTwo();
foo.VerifyAll();
/
* Does not throw MockVerificationException /var foo = new Mock(MockBehavior.Strict);
foo.Setup(f => f.DoOne());foo.Setup(f => f.DoTwo());
foo.DoTwo();foo.DoOne();
foo.VerifyAll();
/
* Does not throw MockVerificationException /var foo = new Mock(MockBehavior.Strict);var seq = new MockSequence();
foo.InSequence(seq).Setup(f => f.DoOne());foo.InSequence(seq).Setup(f => f.DoTwo());
foo.DoOne();foo.DoTwo();
foo.VerifyAll();
/
* Throws MockVerificationException */var foo = new Mock(MockBehavior.Strict);var seq = new MockSequence();
foo.InSequence(seq).Setup(f => f.DoOne());foo.InSequence(seq).Setup(f => f.DoTwo());
foo.DoTwo();foo.DoOne();
foo.VerifyAll();


Reply to this email directly or view it on GitHubhttps://github.com/Moq/moq4/issues/75#issuecomment-30927589
.

Member

kzu commented Dec 19, 2013

But isn't that what verifying the sequence achieves?

/kzu from mobile
On Dec 19, 2013 10:20 AM, "Henrique Esteves" notifications@github.com
wrote:

In my opinion, regardless if you are using a sequence or not, the behavior
of a strict mock must be the same. If "VerifyAll" is called it should check
if all methods were called.

When using a sequence, the "VerifyAll" method should be more restrictive,
not less, as in the example above. Besides checking all the methods were
called, it should check whether they were called in the correct order.

public interface IFoo{
void DoOne();
void DoTwo();}
/* * Does not throw MockVerificationException /var foo = new Mock(MockBehavior.Strict);
foo.Setup(f => f.DoOne());foo.Setup(f => f.DoTwo());
foo.DoOne();foo.DoTwo();
foo.VerifyAll();
/
* Does not throw MockVerificationException /var foo = new Mock(MockBehavior.Strict);
foo.Setup(f => f.DoOne());foo.Setup(f => f.DoTwo());
foo.DoTwo();foo.DoOne();
foo.VerifyAll();
/
* Does not throw MockVerificationException /var foo = new Mock(MockBehavior.Strict);var seq = new MockSequence();
foo.InSequence(seq).Setup(f => f.DoOne());foo.InSequence(seq).Setup(f => f.DoTwo());
foo.DoOne();foo.DoTwo();
foo.VerifyAll();
/
* Throws MockVerificationException */var foo = new Mock(MockBehavior.Strict);var seq = new MockSequence();
foo.InSequence(seq).Setup(f => f.DoOne());foo.InSequence(seq).Setup(f => f.DoTwo());
foo.DoTwo();foo.DoOne();
foo.VerifyAll();


Reply to this email directly or view it on GitHubhttps://github.com/Moq/moq4/issues/75#issuecomment-30927589
.

@henriquemotaesteves

This comment has been minimized.

Show comment
Hide comment
@henriquemotaesteves

henriquemotaesteves Dec 19, 2013

I'm sorry, I accidentally removed the previous comment.

I had written something like this.

In my opinion, regardless if you are using a sequence or not, the behavior of a strict mock must be the same. If "VerifyAll" is called it should check if all methods were called.

When using a sequence, the "VerifyAll" method should be more restrictive, not less, as in the example above. In addition to check if all the methods were called, it should check whether they were called in the correct order.

public interface IFoo
{
    void DoOne();
    void DoTwo();
}

/*
 * Does not throw MockVerificationException
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);

foo.Setup(f => f.DoOne());
foo.Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();
obj.DoTwo();

foo.VerifyAll();

/*
 * Does not throw MockVerificationException
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);

foo.Setup(f => f.DoOne());
foo.Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();
obj.DoTwo();

foo.VerifyAll();

/*
 * Does not throw MockVerificationException
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);
var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.DoOne());
foo.InSequence(seq).Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();
obj.DoTwo();

foo.VerifyAll();

/*
 * Throws MockVerificationException
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);
var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.DoOne());
foo.InSequence(seq).Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoTwo();
obj.DoOne();

foo.VerifyAll();

I'm sorry, I accidentally removed the previous comment.

I had written something like this.

In my opinion, regardless if you are using a sequence or not, the behavior of a strict mock must be the same. If "VerifyAll" is called it should check if all methods were called.

When using a sequence, the "VerifyAll" method should be more restrictive, not less, as in the example above. In addition to check if all the methods were called, it should check whether they were called in the correct order.

public interface IFoo
{
    void DoOne();
    void DoTwo();
}

/*
 * Does not throw MockVerificationException
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);

foo.Setup(f => f.DoOne());
foo.Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();
obj.DoTwo();

foo.VerifyAll();

/*
 * Does not throw MockVerificationException
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);

foo.Setup(f => f.DoOne());
foo.Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();
obj.DoTwo();

foo.VerifyAll();

/*
 * Does not throw MockVerificationException
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);
var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.DoOne());
foo.InSequence(seq).Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();
obj.DoTwo();

foo.VerifyAll();

/*
 * Throws MockVerificationException
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);
var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.DoOne());
foo.InSequence(seq).Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoTwo();
obj.DoOne();

foo.VerifyAll();
@henriquemotaesteves

This comment has been minimized.

Show comment
Hide comment
@henriquemotaesteves

henriquemotaesteves Dec 19, 2013

The previous examples were not very good because I've just described the behavior that is already correct.

The following example demonstrates how a "MockSequence" causes the "VerifyAll" method to be less restrictive, while in my opinion, it should be more restrictive.

/*
 * Does not throw MockVerificationException, but in my opinion it should
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);
var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.DoOne());
foo.InSequence(seq).Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();

foo.VerifyAll();

The previous examples were not very good because I've just described the behavior that is already correct.

The following example demonstrates how a "MockSequence" causes the "VerifyAll" method to be less restrictive, while in my opinion, it should be more restrictive.

/*
 * Does not throw MockVerificationException, but in my opinion it should
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);
var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.DoOne());
foo.InSequence(seq).Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();

foo.VerifyAll();
@kzu

This comment has been minimized.

Show comment
Hide comment
@kzu

kzu Dec 19, 2013

Member

there's a key issue here that I think is missing: a sequence can be used
across several mocks.

So any single mock in that sequence will not be able to verify that it was
called before/after any of the calls in the sequence for the other mocks.

I believe that's a great powerful feature of sequences as implemented
currently, and I can't see how we could add this verification behavior
without introducing a very confusing behavior for cross-mocks verification.

I think verifying the sequence when you have sequenced calls is the right
thing to do.

/kzu

Daniel Cazzulino

On Thu, Dec 19, 2013 at 1:40 PM, Henrique Esteves
notifications@github.comwrote:

The previous examples were not very good because I've just described the
behavior that is already correct.

The following example demonstrates how a "MockSequence" causes the
"VerifyAll" method to be less restrictive, while in my opinion, it should
be more restrictive.

/*

  • Does not throw MockVerificationException, but in my opinion it should
    */
    var foo = new Mock(MockBehavior.Strict);
    var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.DoOne());
foo.InSequence(seq).Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();

foo.VerifyAll();


Reply to this email directly or view it on GitHubhttps://github.com/Moq/moq4/issues/75#issuecomment-30944013
.

Member

kzu commented Dec 19, 2013

there's a key issue here that I think is missing: a sequence can be used
across several mocks.

So any single mock in that sequence will not be able to verify that it was
called before/after any of the calls in the sequence for the other mocks.

I believe that's a great powerful feature of sequences as implemented
currently, and I can't see how we could add this verification behavior
without introducing a very confusing behavior for cross-mocks verification.

I think verifying the sequence when you have sequenced calls is the right
thing to do.

/kzu

Daniel Cazzulino

On Thu, Dec 19, 2013 at 1:40 PM, Henrique Esteves
notifications@github.comwrote:

The previous examples were not very good because I've just described the
behavior that is already correct.

The following example demonstrates how a "MockSequence" causes the
"VerifyAll" method to be less restrictive, while in my opinion, it should
be more restrictive.

/*

  • Does not throw MockVerificationException, but in my opinion it should
    */
    var foo = new Mock(MockBehavior.Strict);
    var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.DoOne());
foo.InSequence(seq).Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();

foo.VerifyAll();


Reply to this email directly or view it on GitHubhttps://github.com/Moq/moq4/issues/75#issuecomment-30944013
.

@henriquemotaesteves

This comment has been minimized.

Show comment
Hide comment
@henriquemotaesteves

henriquemotaesteves Jan 2, 2014

Based on previous comments we have two issues:

1 - The behavior of the "VerifyAll" method of a strict mock in particular when a "MockSequence" is used.
2 - How to verify that a sequence of calls involving different mocks was respected.

Regarding the issue 1, I still think that a "MockSequence" should not interfere with the behavior of the "VerifyAll" method of a strict mock in particular. In the example below an exception should be thrown because a method ("DoTwo") was not invoked, regardless if the sequence was respected or not.

/*
 * Does not throw MockVerificationException, but in my opinion it should
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);
var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.DoOne());
foo.InSequence(seq).Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();

foo.VerifyAll();

Regarding the issue 2, I agree with you. It would be useful if a "MockSequence" had a method, like "VerifyAll", to verify that a sequence of calls involving different mocks was respected.

Based on previous comments we have two issues:

1 - The behavior of the "VerifyAll" method of a strict mock in particular when a "MockSequence" is used.
2 - How to verify that a sequence of calls involving different mocks was respected.

Regarding the issue 1, I still think that a "MockSequence" should not interfere with the behavior of the "VerifyAll" method of a strict mock in particular. In the example below an exception should be thrown because a method ("DoTwo") was not invoked, regardless if the sequence was respected or not.

/*
 * Does not throw MockVerificationException, but in my opinion it should
 */
var foo = new Mock<IFoo>(MockBehavior.Strict);
var seq = new MockSequence();

foo.InSequence(seq).Setup(f => f.DoOne());
foo.InSequence(seq).Setup(f => f.DoTwo());

var obj = foo.Object;

obj.DoOne();

foo.VerifyAll();

Regarding the issue 2, I agree with you. It would be useful if a "MockSequence" had a method, like "VerifyAll", to verify that a sequence of calls involving different mocks was respected.

@kzu

This comment has been minimized.

Show comment
Hide comment
@kzu

kzu Jan 2, 2014

Member

Yup, I agree on both issues.

Member

kzu commented Jan 2, 2014

Yup, I agree on both issues.

@udlose

This comment has been minimized.

Show comment
Hide comment
@udlose

udlose Feb 16, 2017

So is there a way to verify on a MockSequence across all mocks that use that sequence? I'm using v4.5.30 and I don't see it. I was hoping that since this thread was from 3 yrs ago maybe it would have been added by now?

udlose commented Feb 16, 2017

So is there a way to verify on a MockSequence across all mocks that use that sequence? I'm using v4.5.30 and I don't see it. I was hoping that since this thread was from 3 yrs ago maybe it would have been added by now?

@cmeeren

This comment has been minimized.

Show comment
Hide comment
@cmeeren

cmeeren Mar 6, 2017

I'm also wondering if there is a way to verify called sequences across mocks.

cmeeren commented Mar 6, 2017

I'm also wondering if there is a way to verify called sequences across mocks.

@georgiosd

This comment has been minimized.

Show comment
Hide comment
@georgiosd

georgiosd May 12, 2017

Nothing four years later? :(

Nothing four years later? :(

@georgiosd

This comment has been minimized.

Show comment
Hide comment
@Ray-B

This comment has been minimized.

Show comment
Hide comment
@Ray-B

Ray-B Sep 27, 2017

Also would love to see VerifyAll() for MockSequence.

Until then, another alternative approach is to use callbacks() for verifying sequence across multiple mocks:
StackOverFlow Example

Then you don't need to drag YAP (yet another package) into your project.

Ray-B commented Sep 27, 2017

Also would love to see VerifyAll() for MockSequence.

Until then, another alternative approach is to use callbacks() for verifying sequence across multiple mocks:
StackOverFlow Example

Then you don't need to drag YAP (yet another package) into your project.

@stakx

This comment has been minimized.

Show comment
Hide comment
@stakx

stakx Sep 28, 2017

Member

I'd like to suggest the following changes:

  • Let's make sure a MockSequence itself has Verify[All] methods.

  • If mock partakes in a MockSequence, and it is the only mock in that sequence, then mock.Verify[All] simply delegates verification to the sequence.

  • If mock partakes in a MockSequence, but that sequence spans across several different mocks, then mock.Verify[All] fails with an InvalidOperationException. (This is a breaking change, but would alert users to possibly incorrect usage.)

Any thoughts on this?

Member

stakx commented Sep 28, 2017

I'd like to suggest the following changes:

  • Let's make sure a MockSequence itself has Verify[All] methods.

  • If mock partakes in a MockSequence, and it is the only mock in that sequence, then mock.Verify[All] simply delegates verification to the sequence.

  • If mock partakes in a MockSequence, but that sequence spans across several different mocks, then mock.Verify[All] fails with an InvalidOperationException. (This is a breaking change, but would alert users to possibly incorrect usage.)

Any thoughts on this?

@udlose

This comment has been minimized.

Show comment
Hide comment
@udlose

udlose Sep 29, 2017

  1. i like it

  2. i'd argue that an exception should be thown if only one Mock is plced into a MockSequence

  3. i like it. Communication to users about API misuse is becoming increasingly important!

udlose commented Sep 29, 2017

  1. i like it

  2. i'd argue that an exception should be thown if only one Mock is plced into a MockSequence

  3. i like it. Communication to users about API misuse is becoming increasingly important!

@stakx

This comment has been minimized.

Show comment
Hide comment
@stakx

stakx Oct 5, 2017

Member

@udlose:

  1. i'd argue that an exception should be thown if only one Mock is [placed] into a MockSequence

I guess for consistency's sake with my third bullet point? My reasoning for suggesting that this would simply delegate was to have one less breaking change, but I think you're right in principle. I'd happy with either behaviour.

Member

stakx commented Oct 5, 2017

@udlose:

  1. i'd argue that an exception should be thown if only one Mock is [placed] into a MockSequence

I guess for consistency's sake with my third bullet point? My reasoning for suggesting that this would simply delegate was to have one less breaking change, but I think you're right in principle. I'd happy with either behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment