Giter Club home page Giter Club logo

Comments (40)

cmeeren avatar cmeeren commented on July 29, 2024 4

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

from moq4.

henriquemotaesteves avatar henriquemotaesteves commented on July 29, 2024 1

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.

from moq4.

georgiosd avatar georgiosd commented on July 29, 2024 1

Found this: https://github.com/dwhelan/Moq-Sequences
PR with updates here: https://github.com/georgiosd/Moq-Sequences

from moq4.

stakx avatar stakx commented on July 29, 2024 1

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?

from moq4.

udlose avatar udlose commented on July 29, 2024 1
  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!

from moq4.

stakx avatar stakx commented on July 29, 2024 1

I'm not sure which one you're referring to.

The one that's been discussed in the latter half of this issue, where you register mocks with a sequence object, then call verify on the sequence.

Is there a central place where all setups/invocations are registered [...]

No, setups & invocations are recorded on the mocks to which they belong (setups) / on which they occur (invocations).

from moq4.

weitzhandler avatar weitzhandler commented on July 29, 2024 1

I really really appreciate your patience and time writing up such a thorough and explanatory post and I actually agree with all you said.

I'll be experimenting with it and see how it works out.

from moq4.

kzu avatar kzu commented on July 29, 2024

Good point. What should be the expected behavior?

/kzu from mobile
On Dec 18, 2013 7:33 PM, "Henrique Esteves" [email protected]
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//issues/75
.

from moq4.

kzu avatar kzu commented on July 29, 2024

But isn't that what verifying the sequence achieves?

/kzu from mobile
On Dec 19, 2013 10:20 AM, "Henrique Esteves" [email protected]
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//issues/75#issuecomment-30927589
.

from moq4.

henriquemotaesteves avatar henriquemotaesteves commented on July 29, 2024

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();

from moq4.

henriquemotaesteves avatar henriquemotaesteves commented on July 29, 2024

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();

from moq4.

kzu avatar kzu commented on July 29, 2024

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
[email protected]:

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//issues/75#issuecomment-30944013
.

from moq4.

kzu avatar kzu commented on July 29, 2024

Yup, I agree on both issues.

from moq4.

udlose avatar udlose commented on July 29, 2024

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?

from moq4.

georgiosd avatar georgiosd commented on July 29, 2024

Nothing four years later? :(

from moq4.

Ray-B avatar Ray-B commented on July 29, 2024

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.

from moq4.

stakx avatar stakx commented on July 29, 2024

@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.

from moq4.

stakx avatar stakx commented on July 29, 2024

Closing this dormant issue, but marking it as "unresolved" so it can be easily found again. Please see #642 for details. If you'd like to pick this up and work on it, please post here briefly and we'll see what we can do!

from moq4.

TinkerPhu avatar TinkerPhu commented on July 29, 2024

Being a newbie in this blog I might not see the complete picture. But I think testing with sequences is essential.
The moq functionality without sequence was really just a playground for testing but if you want to test protocols and issues during communication you need to be able to use sequences.

@stakx: your proposal goes into the right direction but I think the mock.Verify() and the sequence.Verify() function should be independent. And there is no need for exceptions to educate the user.

  1. mock.Verify() keeps its semantics unchanged and independent of involved sequences by testing whether the set-up calls have been called at least once. These are non vital calls for sequences.

  2. sequence.Verify() shall verify all set-up calls done with InSequence(sequence) on this very sequence. (including order and whether all calls of the sequence have actually been executed)

  3. no exceptions for sequence.Verify() if no call / mock is involved at all or if it is only one. I think there is no necessity to force more than one mock or any at all. The point is to verify an order of 0...N calls setup by 0..N mocks.

Inspired by this post ( https://stackoverflow.com/questions/32585355/trying-to-understand-mocksequence ) I continued its idea which I would prefer to have completed in Moq:

public class SequenceVerifyer
{
    public MockSequence Sequence { get; private set; } = new MockSequence();

    public Action NextCallback()
    {
        var callNo = setupCount++;
        return () => { AssertCallNo(callNo);};
    }
    public void VerifyAll()
    {
        Assert.AreEqual(setupCount, executionCount, $"less calls ({executionCount}) executed than previously set up ({setupCount}).");
    }

    private int executionCount = 0;
    private int setupCount = 0;

    private void AssertCallNo(int expectedCallNo)
    {
        Assert.AreEqual(executionCount, expectedCallNo, $"order of call is wrong. this call is marked with No ({expectedCallNo}), but ({executionCount}) was expected.");
        executionCount++;
    }
}

usage:

public interface IFoo
{
    bool foo(int n);
}

public interface IBar
{
    int bar(string a);
}

[Test]
public void SequenceVerifyer_with_full_sequence()
{
    var fooMock = new Mock<IFoo>(MockBehavior.Strict);
    var barMock = new Mock<IBar>(MockBehavior.Strict);
    var seq = new SequenceVerifyer();

    fooMock.Setup(f => f.foo(3)).Returns(false);
    barMock.Setup(b => b.bar("world")).Returns(4);

    fooMock.InSequence(seq.Sequence).Setup(f => f.foo(4)).Returns(true).Callback(seq.NextCallback());
    barMock.InSequence(seq.Sequence).Setup(b => b.bar("hello")).Returns(2).Callback(seq.NextCallback());
    fooMock.InSequence(seq.Sequence).Setup(f => f.foo(4)).Returns(false).Callback(seq.NextCallback());

    fooMock.Object.foo(3); //non sequence
    fooMock.Object.foo(4);
    barMock.Object.bar("hello");
    barMock.Object.bar("world"); //non sequence
    fooMock.Object.foo(4);

    fooMock.VerifyAll();
    barMock.VerifyAll();
    seq.VerifyAll();
}

from moq4.

weitzhandler avatar weitzhandler commented on July 29, 2024

In my scenario, I'm using non-strict mocks.
I just want to use the sequence in the verification stage, so that the calls to the various underlying services were made by the SUT in the correct order.

One way to achieve that is to create a disposable sequence, maybe such that all mocks are added to it, that collects data about underlying calls and underlying verifications, then compares the order of the calls to the order of verifications.

I'm willing to implement this feature.

[Fact]
public void Test1()
{
  // arrange
  var jobServiceMock = new Mock<IJobService>();
  var workServiceMock = new Mock<IWorkService>();

  using var sequence = Sequence.Create(jobServiceMock, workServiceMock); //maybe no need to provide mocks

  var sut = new SystemUnderTest(jobServiceMock.Object, workService.Object);

  // act
  sut.DoWorkAndJob();

  // assert             
  workServiceMock.Verify(work => work.DoWork(), Times.Once);
  jobServiceMock.Verify(job => job.DoJob(), Times.Once);

  sequence.Dispose(); //redundant - verifies recorded calls match their verification order
}

from moq4.

weitzhandler avatar weitzhandler commented on July 29, 2024

Otherwise, VerifyInSequence instead of Verify, something like:

[Fact]
public void Test1()
{
  // arrange
  var jobServiceMock = new Mock<IJobService>();
  var workServiceMock = new Mock<IWorkService>();

  using var sequence = Sequence.Create(jobServiceMock, workServiceMock); //maybe no need to provide mocks

  var sut = new SystemUnderTest(jobServiceMock.Object, workService.Object);

  // act
  sequence.Execute(() => sut.DoWorkAndJob());

  // assert             
  workServiceMock.VerifyInSequence(sequence, work => work.DoWork(), Times.Once);
  jobServiceMock.VerifyInSequence(sequence, job => job.DoJob(), Times.Once);

  sequence.Dispose(); //redundant - verifies recorded calls match their verification order
}

from moq4.

stakx avatar stakx commented on July 29, 2024

Thanks for your willingness to work on this @weitzhandler.

I have to say, however, that ambient sequences (the API approach taken by the 3rd party package Moq.Sequences) can be problematic for a variety of reasons, it would perhaps be better to have something more explicit; such as creating a sequence, then registering mocks with it, then in the end calling verification methods on the sequence.

VerifyInSequence would be another possibile design, we'd then likely have to record a steadily increasing sequence number (think a tick of a global virtual clock) with every invocation so we know in which order they happened. You'd still have to pass it a sequence object because calling VerifyInSequence obviously needs to keep some state (at least, the clock tick value of the invocation last verified in one isolated test).

I'm personally gravitating to the first of the two designs just mentioned (as it has already been discussed further above and got some feedback, also we wouldn't need new method names).

from moq4.

weitzhandler avatar weitzhandler commented on July 29, 2024

I'm personally gravitating to the first of the two designs just mentioned

I'm not sure which one you're referring to.

from moq4.

weitzhandler avatar weitzhandler commented on July 29, 2024

Is there a central place where all setups/invocations are registered or can be intercepted from?

from moq4.

weitzhandler avatar weitzhandler commented on July 29, 2024

No, setups & invocations are recorded on the mocks to which they belong (setups) / on which they occur (invocations).

I'm wondering if there is a way to get a notification when Mock.Invocations updates, any chance to turn this collection into an INotifyCollectionChanged or similar, or introduce a Mock.InvocationOccurred event? Adding a timestamp property to Invocation?
Any other way to intercept an invocation occurrence? Can we "intercept the interceptors"?

Please consider reopening this issue, or should I post a new issue as my request is different, my request is allowing sequence verification for loose mocks.

from moq4.

stakx avatar stakx commented on July 29, 2024

The approach that has been discussed in this issue before does not require any events being added to mock.Invocations, nor does it require timestamps. All state would be kept in MockSequence, i.e. you'd only pay (performance wise) for what you need... which is another reason why I personally favor that approach.

from moq4.

weitzhandler avatar weitzhandler commented on July 29, 2024

But that's only possible for strict setup, did I miss something?

from moq4.

stakx avatar stakx commented on July 29, 2024

You'd still have to register mocks with the sequence—something along the lines of mock.InSequence(mockSequence)—so that Moq knows to record all of mock's invocations in mockSequence, too.

from moq4.

weitzhandler avatar weitzhandler commented on July 29, 2024

You'd still have to register mocks with the sequence, so that their invocations can be recorded there.

Given a mock, is there a way to listen and intercept its invocations as they happen? Because if it's possible, is easy to record them with a timestamp or chronologically.
What's the magic work to look for, I'm a Castle noob, it'll help me tremendously if you give me a keyword or a reference to look for.

something along the lines of mock.InSequence(mockSequence)—so that Moq knows to record all of mock's invocations in mockSequence, too.

Oops, I see your update now.
So, could I write using var seq = new Sequence(mock1, mock2, mock3); instead of mock1.InSequence(seq); mock2.InSequence(seq); mock3.InSequence(seq);? Would you consider that?

Thank you for everything @stakx!

from moq4.

stakx avatar stakx commented on July 29, 2024

Given a mock, is there a way to listen and intercept its invocations as they happen?

No, but that shouldn't be necessary. It would perhaps be easiest if mocks actively cooperated with sequences and recorded their invocations there.

What's the magic work to look for, I'm a Castle noob, it'll help me tremendously if you give me a keyword or a reference to look for.

It shouldn't be necessary to descend into the parts of Moq that deal with Castle DynamicProxy. Moq builds its own abstractions on top. Moq's method invocation interception pipeline can be found there. I should probably give you a few more details how we should hook it up with MockSequence but until then take a look if you'd like.

One word of caution: it's good to keep in mind that all code we add in the interception pipeline will run for every single method that gets called on mocks. That's why it's somewhat important to keep that code as slim as possible. I'm not saying we need to micro-optimise (though I've been guilty of doing just that in the past 😉) but just to stay aware of performance in that part of Moq.

using var seq = new Sequence(mock1, mock2, mock3); instead of mock1.InSequence(seq);

(I'm going to assume you really meant MockSequence, not Sequence.)

I haven't given that too much thought yet, TBH. I suppose that, in order to keep in line with the existing API, it would be nice if you could register mocks to an already existing sequence (similar to how you can keep adding setups to a mock), which wouldn't be possible if we did the ctor syntax. Let me get back to you on that point.

from moq4.

weitzhandler avatar weitzhandler commented on July 29, 2024

No, but that shouldn't be necessary. It would perhaps be easiest if mocks actively cooperated with sequences and recorded their invocations there.

Ok. I guess then it'll be fairly easy to implement my suggestion above (VerifyInSequence).

But do you think the following could also be achievable:

// arrange
var sut = new SystemUnderTest(mock1, mock2, ...);

using var seq = new MockSequence(mock1, mock2, ...); // as soon as we add to sequence, all verifications on this mock 
                                                     // are now intercepted by the sequence too

// act
sut.DoWork();

// assert
mock1.Verify(service1 => service1.DoWork(), Times.Once); // mocks are automatically tracked by sequence
mock2.Verify(service2 => service2.DoWork(), Times.Once);

Is there a way to "intercept" verifications on a mock? Or do you even like this way?

Ideally my goal would be:

  • Setup a sequence once with all mocks
  • No need for mock setup
  • Regular verification on mock triggers sequence verification, if mock is in sequence
    One way to achieve this, is having a single/list of sequences in the mock itself, so that that it notifies the sequence about an invocation/verification. Though not a great design I think.

from moq4.

stakx avatar stakx commented on July 29, 2024

But do you think the following could also be achievable:

Yes, it would be achievable.

Or do you even like this way?

Personally, no, I don't think the design you're proposing is optimal. I'll try to give you a few reasons why I think so.

Ambient Context as an anti-pattern. I briefly mentioned above that I see "ambient" sequences as problematic "for a variety of reasons". By "ambient", I mean that the mere act of instantiating a MockSequence is enough to alter the behavior of the remaining other code—even if the sequence object is never referenced anymore thereafter... it's just hovering around in the background. For example, mock.Verify would suddenly acquire a different meaning / mode of operation because of the presence of another object that you might not even directly see in your test method's code. Altered code behavior through a possibly unseen object is one of the various reasons I alluded to above. It could make test code harder to understand at a glance.

(In a similar vein, I would say the Mock.Switches property that I am responsible for was a bad addition to Moq's API, not just because it's not really necessary, but also for its "ambient" character: feature flags can change code behavior while being relatively "invisible".)

I do grant, however, that ambient contexts can feel more convenient than more explicit code.

Possibly too coarse-grained. Your design, as I currently understand it, doesn't account for the possible need to keep mock verification and mock sequence verification apart. Say you want to verify that some invocation on a mock happened, but it doesn't matter when in the order of all invocations? Not possible if that mock participates in a sequence.

This may seem somewhat academic (i.e. like an unrealistic use case), but never underestimate what use cases users will come up with. :-)

Extensibility. Other design limitations become apparent when you consider other testing approaches. What if one prefers strict mocks or setups over simple call verifications? What if you want to allow cyclic sequences (like e.g. what the Moq.Sequences library has)?

Now let me propose an alternate design—basically the one that's been previously discussed in this issue—that circumvents most if not all of the above problems:

partial class MockSequence
{
    public ISequenceInvocationList Invocations { get; }
    public ISequenceMockList Mocks { get; }
    public MockSequence(params Mock[] mocks);  // or alternatively, `mockN.InSequence()`
    public void Verify<TMock>(Expression<Action<TMock>> invocation, Times times = default, string failMessage = default, ...);
    public void Verify<TMock, TReturn>(Expression<Func<TMock, TReturn>> invocation, Times times = default, string failMessage = default, ...);
    public void VerifyNoOtherCalls();
}

partial interface ISequenceMockList : IReadOnlyList<Mock> { }

partial interface ISequenceInvocationList : IReadOnlyList<ISequenceInvocation> { }

partial interface ISequenceInvocation : IInvocation
{
    Mock Mock { get; }
}

(Not all of this would have to be implemented right away. At the core, we'd need the Verify method group, and some way to enlist one or more mocks in a sequence. We already have mock.InSequence() which could serve for that purpose.)

Let's look at what changes with regard to the issues raised above:

  • There is nothing ambient here, there's no hidden magic: You instantiate a sequence, add mocks to it, you have a record of all mock-specific invocations that happen, and you verify directly on the sequence if that is what you intend. All of this has to happen explicitly in your test code, or it's not going to happen.

  • You can verify calls that happen on mocks without verifying them on the mock sequence.

  • It's extensible, because basically the mock sequence API has the same basic shape as the mock API. It would be easy to extend mock sequences so they are strict, i.e. allow only invocations that have been previously set up. It would be straightforward to add Setup methods to MockSequence. Those probably wouldn't need any of the usual verbs like .Callback, .Throws, .Returns, etc. as you'd typically configure those on the mocks directly—but it would be possible to add even those, too. (The difference would be that they'd only take effect if an invocation happens at the right time in the configured order.)

So now that I've laid out a few arguments perhaps you can see why my personal preference is if we followed the previous trajectory of this issue, where we add .Verify methods to MockSequence and keep the "ambient" magic out of the picture.

from moq4.

tonyhallett avatar tonyhallett commented on July 29, 2024

Please see

VerifiableSequence
MockSequencePhraseBase

Mock is still ignorant of sequences.
Custom sequences can now be created - MockSequence
Base sequence provides verification behaviour that can be overridden - VerifySequence
Verifcation of sequence and mock/mocks. Verification of multiple sequences.
VerifyAll as mentioned in this issue - VerifiableSequenceExtensions
Works for mocks and protected mocks - Protected mocks
Needs some tidying up but ...

from moq4.

stakx avatar stakx commented on July 29, 2024

@tonyhallett – thanks for your interest in picking this up. I've glanced over the bits of implementation code you've linked to above, but it's a little difficult to see how it would all fit together from a user's point of view.

What I can tell so far is that you seem to be adding a lot of new parts to the public API. At least some of that (all those verification methods for various combinations of mock / setups) looks redundant... is all of that really necessary?

As I already outlined above, I would personally prefer an approach that adds as little as possible; and the bits that get added would ideally be similar to what we already have elsewhere. IMHO, sequences could be made much more powerful already just by adding Verify[All] methods to MockSequence (and if we're generous, perhaps VerifyNoOtherCalls as well as an additional ctor MockSequence(MockBehavior)).

(Unfortunately, I cannot invest a lot of time into Moq at the moment, so apologies in advance if I'll be a little slow in responding here... I'll be instead focusing on getting the recent changes released soon.)

from moq4.

tonyhallett avatar tonyhallett commented on July 29, 2024

What I can tell so far is that you seem to be adding a lot of new parts to the public API. At least some of that (all those verification methods for various combinations of mock / setups) looks redundant... is all of that really necessary?

Having hidden some of the internal methods ( the solution is just a quick and dirty idea for discussion ) the public api consists of

Verifying the setups on the sequence - so VerifySequence could be

sequences could be made much more powerful already just by adding Verify[All] methods to MockSequence

Helper methods for further verification
MockVerify / MockVerifyAll - these fulfill the behaviour desired by @henriquemotaesteves - mock.VerifyAll
that the setups performed in the sequence are included ( but not for setups in the sequence for other mocks )
but are ignored by the isConditional check
https://github.com/moq/moq4/blob/9e148f6955a2aef09c6e4a5c117c7c7deef0756e/src/Moq/Mock.cs#L301

VerifySequenceAndAllMocks ( all mocks in the sequence ) - equalivalent of a "working" mock.Verify/VerifyAll for all mocks in the sequence.

Other verification methods could go.

The other part of the public api is only relevant if you are creating your own definition of a sequence.

The base class facilitates this by providing to derivations the ISetup ( the root one - not the one for inner mocks ) that can be verfied and an index representing the order of the setup within the setups performed on the sequence, combined with the virtual AddedSetup method it is possible for a derivation to "set up" a sequence with different behaviour. As the behaviour has changed to support any definition of a sequence the implementation is supplied the corresponding setup for the Condition that determines if there is a match.
If the Condition is true then we can mark the setup with an invocation index and it is this ITrackedSetup that can be used for custom sequence verification by a derivation ( or even by a user if desired ).

So ultimately I would say that I am adding two parts from a user's point of view
verification like mock.Verify/VerifyAll
the idea of an extensible sequence.

it's a little difficult to see how it would all fit together from a user's point of view.

Continue as before accept it is now possible to "VerifyAll"
If you find MockSequence is not sufficient for describing your sequence then create your own.
https://github.com/moq/moq4/blob/43e7e872fdd0933ab2b921e8957cd502fc0889df/tests/Moq.Tests/MockSequenceFixture.cs#L443

This leads us to what it does not do

The sequence does not automatically track invocations on the registered mocks as per your design.

partial class MockSequence
{
    public ISequenceInvocationList Invocations { get; }
    public ISequenceMockList Mocks { get; }
    public MockSequence(params Mock[] mocks);  // or alternatively, `mockN.InSequence()`
    public void Verify<TMock>(Expression<Action<TMock>> invocation, Times times = default, string failMessage = default, ...);
    public void Verify<TMock, TReturn>(Expression<Func<TMock, TReturn>> invocation, Times times = default, string failMessage = default, ...);
    public void VerifyNoOtherCalls();
}

Questions about your design ( tbc )...

from moq4.

stakx avatar stakx commented on July 29, 2024

@tonyhallett, it's obvious that you've put quite a bit of thought into this... I'll probably need some time to digest all of it. I'll probably move your above posts to a separate issue so that we can discuss it in isolation–this issue is getting way too big. :-D

I'll get back to this soon.

from moq4.

tonyhallett avatar tonyhallett commented on July 29, 2024

@stakx I will post another solution tomorrow that is an improvement upon the current one.

from moq4.

tonyhallett avatar tonyhallett commented on July 29, 2024

@stakx #1193
Please ignore as closed.

from moq4.

tonyhallett avatar tonyhallett commented on July 29, 2024

@stakx Should have a solution tomorrow worth looking at.

from moq4.

MichaelKetting avatar MichaelKetting commented on July 29, 2024

@stakx I've just stumbled over this issue while migrating my tests from RhinoMocks to Moq and realizing that .InSequence(...) + .Verify() isn't actually giving me all the failure scenarios I require. Thank you also for the detailed blog post on this subject (https://stakx.github.io/dissecting-moq4/2020/12/26/mock-sequence.html)

@tonyhallett thank you for all the work you've put into this so far!

I've looked at the API in #1197 and believe it would be helpful to be able to register a mock after creating the sequence. I haven't checked all my code for this yet, but there's some places where creating the mock after the sequence was created is a scenario I do have.

from moq4.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.