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

Setup/Verifyable With Times #530

Closed
BlythMeister opened this issue Nov 23, 2017 · 7 comments
Closed

Setup/Verifyable With Times #530

BlythMeister opened this issue Nov 23, 2017 · 7 comments

Comments

@BlythMeister
Copy link
Contributor

BlythMeister commented Nov 23, 2017

Would it be possible to provide a setup with an expected number of times the execution occurs.

And then use verifyable and verify all to ensure that it was executed the correct number of times.

This would work in a similar way to RhinoMocks with

var mock = MockRepository.GenerateStrictMock<IDoStuff>();
var sut = new FunctionRunner(mock);

mock.Expect(x => x.DoFunction()).Return(true).Repeat.Once;
mock.Expect(x => x.DoFunction()).Return(false).Repeat.Times(5);

sut .Execute();

mock.VerifyAllExpectations();

There is 2 reasons why you might want to do this.

  1. You might want a different result on the 1st call to all subsequent calls
  2. If your function takes a number of parameters, you don't need to specify a setup and verify which can be very verbose.

Currently in moq you would need to:

var mock = new Mock<IDoStuff>(MockBehavior.Strict);
var sut = new FunctionRunner(mock.Object);

var calls = 0;
mock.Setup(x => x.DoFunction()).Returns(calls==0).Callback(() => calls++);

sut .Execute();

mock.Verify(x => x.DoFunction(), Times.Exactly(6));

If DoFunction took a lot of parameters, or something which is verbose, that's a lot of copied data between the setup and verify, just to make sure it was called x times and returned the right values on each of the 6 calls (true the first time, and false the other 5 times)

If this could be simplified to

var mock = new Mock<IDoStuff>(MockBehavior.Strict);
var sut = new FunctionRunner(mock.Object);

mock.Setup(x => x.DoFunction(), Times.Exactly(1)).Returns(true);
mock.Setup(x => x.DoFunction(), Times.Exactly(5)).Returns(false);

sut .Execute();

mock.VerifyAll();
@stakx
Copy link
Contributor

stakx commented Nov 23, 2017

Duplicate of #373.

@BlythMeister
Copy link
Contributor Author

BlythMeister commented Nov 23, 2017

@stakx sorry, i didn't search closed issues :(

as an asside, for anyone else who comes here, this is my workaround/hack which is a superclass of a Mock which implments a setup and saves the verify to be called as part of the verify all.
Joining the 2 processes together in a single setup call :)

Granted, this doesn't actually go as far as to error in a strict mock on the additional execution, but will error at the end.
It also doesn't help with the setup requiring a callback to change behaviour between 1st and 2nd call.

This is a pure helper so you can setup and verify the same thing with times which for me is a happy middle case.

    public class ExpectantStrictMock<T> : Mock<T> where T : class
    {
        private readonly List<Action> verifyFunctions;

        public ExpectantStrictMock() : base(MockBehavior.Strict)
        {
            verifyFunctions = new List<Action>();
        }

        public ISetup<T> SetupExpect(Expression<Action<T>> action, Times times)
        {
            verifyFunctions.Add(() => Verify(action, times));
            return Setup(action);
        }

        public ISetup<T> SetupExpect(Expression<Action<T>> action)
        {
            return SetupExpect(action, Times.AtLeastOnce());
        }

        public void VerifyAllExpectations()
        {
            foreach (var verifyFunction in verifyFunctions)
            {
                verifyFunction();
            }
        }
    }

@BlythMeister
Copy link
Contributor Author

BlythMeister commented Nov 23, 2017

@stakx having read through #373 I actually don't believe this is a duplicate as the reasoning behind this is not the same.

The point I am trying to make is 2 fold.

The first is the ability to specify a return to only occur X number of times.
Therefore giving the ability to specify different returns on the exact same function call whereby the variance is the number of times executed.

The second is the ability to chain a setup with a verify in a single statement.
Therefore preventing the requirement to specify the mock expression multiple times (once on the setup, and once on the verify)
This reduces the amount of test code and potential rework should you need to refactor function parameters

In summary, the ability to state during setup, I want this to happen when the function is called X times.
This therefore has 2 roles, a limit to number of times a return or callback is executed and a verification of the number of times the function was called.
Both are critical factors in unit testing and whilst this can be achieved using the callback syntax it is rather verbose.

@stakx stakx removed the duplicate label Nov 24, 2017
@stakx
Copy link
Contributor

stakx commented Nov 24, 2017

I don't believe the suggested syntax would be a very good fit with Moq's current API. It would basically turn everything into sequences. (And there are already two mechanisms for that, MockSequence and SetupSequence—did you look at those?)

mock.SetupSequence(m => DoSth())
    .Returns(true)
    .Returns(false)
    .Returns(...)
    ...
    .Throws(new InvalidOperationException());
...

(Yes, SetupSequence is very explicit in requiring concrete values for every single invocation. There's obviously room for improvement there.)

The syntax you're suggesting (Times parameter to Setup meaning that the setup will expire after that amount of calls) would very likely be very difficult if not impossible to implement because (a) noone says that Times is required to be a continuous range of numbers in theory, and (b) it interferes with Moq's principle that a setup with an identical call expression as another, preceding setup overrides that setup).

I don't see any way to implement your suggestion. I think we'd do better to focus on making Moq's sequence features (e.g. SetupSequence or MockSequence) more useful.

P.S.: There are also conditional setups (mock.When(condition).Setup(...)) you could use towards that end. Something lile:

int calls = 0;
mock.When(() => calls == 0).Setup(m => m.DoSth()).Returns(true).Callback(() => ++calls);
mock.When(() => calls < 6).Setup(m => m.DoSth()).Returns(false).Callback(() => ++calls);
...
Assert.Equal(6, calls);

Be aware that conditional setups done this way won't be thread-safe.

@stakx stakx closed this as completed Nov 24, 2017
@BlythMeister
Copy link
Contributor Author

BlythMeister commented Nov 24, 2017

@stakx i didn't know about SetupSequence, that does exactly what i wanted for half of this issue :)

The other half is essentially this.
Is there a way to simplify this example

[TestFixture, Parallelizable(ParallelScope.All)]
    public class MockupTests
    {
        public interface IDoStuff
        {
            void Execute(string key, string key2, string key3, string key4, string key5);
        }

        public class TestSystem
        {
            private readonly IDoStuff doStuff;

            public TestSystem(IDoStuff doStuff)
            {
                this.doStuff = doStuff;
            }

            public void TestFunction()
            {
                doStuff.Execute("Testing", "Testing2", "Testing3", "Testing4", "Testing5");
            }
        }

        private Mock<IDoStuff> mock = new Mock<IDoStuff>(MockBehavior.Strict);
        private TestSystem sut;

        [SetUp]
        public void Setup()
        {
            sut = new TestSystem(mock.Object);
        }

        [Test]
        public void Test()
        {
            mock.Setup(x => x.Execute(It.Is<string>(v => Equals(v, "Testing")), It.Is<string>(v => Equals(v, "Testing2")), It.Is<string>(v => Equals(v, "Testing3")), It.Is<string>(v => Equals(v, "Testing4")), It.Is<string>(v => Equals(v, "Testing5"))));

            sut.TestFunction();

            mock.Verify(x => x.Execute(It.Is<string>(v => Equals(v, "Testing")), It.Is<string>(v => Equals(v, "Testing2")), It.Is<string>(v => Equals(v, "Testing3")), It.Is<string>(v => Equals(v, "Testing4")), It.Is<string>(v => Equals(v, "Testing5"))), Times.Once);
        }
    }

having the setup and verify lines being 90% the same, but verify having the extra condition just seems very verbose and this feels a bit wrong

        [Test]
        public void Test2()
        {
            Expression<Action<IDoStuff>> mockEx = x => x.Execute(It.Is<string>(v => Equals(v, "Testing")), It.Is<string>(v => Equals(v, "Testing2")), It.Is<string>(v => Equals(v, "Testing3")), It.Is<string>(v => Equals(v, "Testing4")), It.Is<string>(v => Equals(v, "Testing5")));

            mock.Setup(mockEx);

            sut.TestFunction();

            mock.Verify(mockEx, Times.Once);
        }

@stakx
Copy link
Contributor

stakx commented Nov 24, 2017

@BlythMeister - #527 is discussing a possibility of avoiding repetition in setup / verification expressions.

Apart from that, the easiest way of avoiding identical setup and verification expressions would be to use a loose mock instead of a strict mock (if that's acceptable in your situation), because then you might not need the setup at all and would only be left with the verification expression.

Another (albeit not very elegant) alternative would be to store the expression in a variable, then pass it to Setup and Verify. (I personally wouldn't want to do this, however.)

@BlythMeister
Copy link
Contributor Author

ok, i'll keep an eye out for #527 thanks.

And yeah, my second example was the not very elegant scenario.
Obviously, it's also possible to do what i suggested earlier in the thread and extend the Mock class

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

No branches or pull requests

2 participants