FakeItEasy icon indicating copy to clipboard operation
FakeItEasy copied to clipboard

A way to define a collection in argument constraint

Open lucasteles opened this issue 7 years ago • 13 comments

Would be nice a new argument matcher for a collection of values, that can be good if we are testing some method that process an argument list

Something like this IsInOrderFromColection

    public interface IDep { Double(int n);  }

     var mock = A.Fake<IDep>();
     A.CallTo(() => mock.Double(A<int>._)).Returns(0);

     var numbers = new[] { 2, 4, 5 };
     foreach (var n in numbers)
        mock.Double(n);

      A.CallTo(() => 
         mock.Double(A<int>.That.IsInOrderFromColection(2, 4, 5) ))
         .MustHaveHappened();

it should verify if the Double method is called with 2, 4 and 5 in sequence,

lucasteles avatar Mar 13 '19 21:03 lucasteles

Hi, @lucasteles. Thanks for raising this. Just to make sure I understand correctly, you're essentially proposing an alternative syntax for

A.CallTo(() => mock.Double(2)).MustHaveHappened()
    .Then(A.CallTo(() => mock.Double(4)).MustHaveHappened())
    .Then(A.CallTo(() => mock.Double(5)).MustHaveHappened());

?

blairconrad avatar Mar 13 '19 21:03 blairconrad

@blairconrad exactly,

Maybe we can have this and an ’IsFromColection' wich is the same idea, but without the ’Then’

lucasteles avatar Mar 13 '19 23:03 lucasteles

Needed that today, ended up using the Then chain, but I agree that having an overload is much more readable.

akamud avatar Mar 14 '19 00:03 akamud

Maybe we can have this and an ’IsFromColection' wich is the same idea, but without the ’Then’

@lucasteles could you elaborate? I'm not sure I understand what you want. What would the usage look like?

thomaslevesque avatar Mar 14 '19 00:03 thomaslevesque

Do you mean that this:

      A.CallTo(() => 
         mock.Double(A<int>.That.IsFromCollection(2, 4, 5) ))
         .MustHaveHappened();

would actually perform 3 assertions ?

If that's what you want, I'm afraid it's going to be very hard to implement... A lot of things would have to change, for a relatively rare use case for which there's an easy workaround. The benefits would certainly not outweigh the cost.

thomaslevesque avatar Mar 14 '19 01:03 thomaslevesque

@thomaslevesque Something like it, in this case the Double method should be called 3 times, with each argument,

I cant say if we will need 3 assertions or if we can have the same behavior with other solution

The motivation behind this is a case when i have a method wich receives a collection, and for each item i call other method from a dependency (my mocked service), and in my test i want to check in my dependency receives each item of my collection in an readable way.

lucasteles avatar Mar 14 '19 01:03 lucasteles

@lucasteles you can just use a loop (if you don't care about the order)

foreach (var n in numbers)
{
    A.CallTo(() => mock.Double(n)).MustHaveHappened();
}

If you do care about the order, you could use this class to do something like this:

var current = NullOrderableCallAssertion.Default;
foreach (var n in numbers)
{
    current = current.Then(
        A.CallTo(() => mock.Double(n)).MustHaveHappened());
}
Off topic @blairconrad I wonder if it would make sense to include `NullOrderableCallAssertion` (or an equivalent feature) in FakeItEasy. It's not something that's commonly needed, but in a scenario like this it's pretty useful.

thomaslevesque avatar Mar 14 '19 01:03 thomaslevesque

@thomaslevesque yeh, that solves the problem, but affect the readability of the test, is not very good have this type of loops on it.

I realy like FakeItEasy for the way of it turns natural to read a test, and i think this solution makes the opposite.

But i understand that this problem is not very common and if it is so hard then cant worth it

lucasteles avatar Mar 14 '19 02:03 lucasteles

@lucasteles, I understand your reticence to include a loop in your test, but it might not be hard to hide it within a method. Hmm. Lemme try something.

I'm back. I hacked this up:

public class MustHaveHappenedForEachArgument
{
    public interface IDep { int Double(int n); }

    [Test]
    public void T()
    {
        var mock = A.Fake<IDep>();
        A.CallTo(() => mock.Double(A<int>._)).Returns(0);

        var numbers = new[] { 2, 4, 5 };
        foreach (var n in numbers)
        {
            mock.Double(n);
        }

        AssertWasCalledWithEach(n => () => mock.Double(n), numbers);
    }

    private static void AssertWasCalledWithEach(Func<int, Expression<Func<int>>> callSpecificationFactory, IEnumerable<int> args)
    {
        var current = NullOrderableCallAssertion.Default;
        foreach (var arg in args)
        {
            current = current.Then( // originally did not assign - thanks, @thomaslevesque 	
                A.CallTo(callSpecificationFactory(arg)).MustHaveHappened());
        }
    }

    private class NullOrderableCallAssertion : IOrderableCallAssertion
    {
        public static IOrderableCallAssertion Default { get; } = new NullOrderableCallAssertion();

        private NullOrderableCallAssertion() { }

        public IOrderableCallAssertion Then(UnorderedCallAssertion nextAssertion)
        {
            return nextAssertion;
        }
    }
}

It's not perfect, but maybe makes things more palatable for you? I tried fully genericizing the method, but the compiler could not figure out the signature.

blairconrad avatar Mar 14 '19 18:03 blairconrad

Huh. That seems to pass when the calls are made in the right order, and also when they aren't! @thomaslevesque, even your original fails, which surprises me:

[Test]
public void FailsLikeItShould()
{
    var mock = A.Fake<IDep>();
    A.CallTo(() => mock.Double(A<int>._)).Returns(0);

    foreach (var n in new[] { 2, 4, 5 })
    {
        mock.Double(n);
    }

    A.CallTo(() => mock.Double(2)).MustHaveHappened()
       .Then(A.CallTo(() => mock.Double(5)).MustHaveHappened())
       .Then(A.CallTo(() => mock.Double(4)).MustHaveHappened());
}

[Test]
public void PassesButShouldNot()
{
    var mock = A.Fake<IDep>();
    A.CallTo(() => mock.Double(A<int>._)).Returns(0);

    foreach (var n in new[] { 2, 4, 5 })
    {
        mock.Double(n);
    }

    var current = NullOrderableCallAssertion.Default;
    foreach (var n in new[] { 2, 5, 4 })
    {
        int arg = n;
        current.Then(
            A.CallTo(() => mock.Double(arg)).MustHaveHappened());
    }
}

I'm at a temporary loss. Might have time to look into it tonight. Until then I don't think we should include NullOrderableCallAssertion in the product! :wink:

blairconrad avatar Mar 14 '19 18:03 blairconrad

@blairconrad sorry, my example was wrong, I fixed it. current needs to be reassigned with the result of Then in the loop body.

thomaslevesque avatar Mar 14 '19 20:03 thomaslevesque

Oh, thanks, @thomaslevesque. :blush: Of course.

blairconrad avatar Mar 14 '19 20:03 blairconrad

I incorporated the fix into https://github.com/FakeItEasy/FakeItEasy/issues/1581#issuecomment-472996795 as well.

blairconrad avatar Mar 14 '19 20:03 blairconrad

There's been no activity in this issue for some time, so we're closing it. We can reopen the issue in the future should sufficient interest arise.

thomaslevesque avatar Apr 18 '24 14:04 thomaslevesque