NSubstitute icon indicating copy to clipboard operation
NSubstitute copied to clipboard

AndDoes runs BEFORE ReturnsForAnyArgs

Open bkqc opened this issue 3 years ago • 3 comments

I have a piece of code that inserts values inside a dictionary for later consumption in the assert. In this code, I have to increment a value and to have it as clean as possible, I did it in the AndDoes call. At first, I had

var index= 1;
mySub.GetValAsync(default).ReturnsForAnyArgs(info => index.ToString())
    AndDoes(ci => compteurRepetitions.Add(index++.ToString(), 0));

The returned values were 2 and 3 and the stored dictionary had 1 and 2

To get the expected value, I needed to modify the function as such:

var index= 1;
mySub.GetValAsync(default).ReturnsForAnyArgs(info => index++.ToString())
    AndDoes(ci => compteurRepetitions.Add(index.ToString(), 0));

which then returned 2 and 3 with stored dictionary values 1 and 2.

Also, when adding breakpoints inside each function, it clearly stops in AndDoes before ReturnsForAnyArgs. This behaviour does not match what is described in the documentation

bkqc avatar May 19 '22 18:05 bkqc

Hi @bkqc ,

AndDoes will run before the Returns*, as the return is the last thing executed in a method. I understand this is not really made clear in the documentation. Do you have any suggestions for how we can clarify this?

dtchepak avatar May 30 '22 09:05 dtchepak

I think there are 2 problems:

  1. The method is called AndDoes which sounds like And Then Does. If the methods were Does(...).AndReturns(...) I think it may have been clearer. The fact that it comes after in the declaration is misleading. In fact, though I suppose this would be a breaking change, though the return is the las thing to occur in a method, AndDoes could have called a delegate of Returns, stored the result, executed its own code and finally returned the value received from the delegate.
  2. The example in the documentation does not use the counter in the return. If it did, I think it may be clearer.

Also, maybe having some specific documentation on AndDoes giving precisions on what it does. In the actual documentation, it is merely mentionned.

bkqc avatar Jul 19 '22 20:07 bkqc

I think updating the doc to show it happens before the .Returns would be nice. The code from the 2013 issue when this was added showed an action occurring at the end of the method altho clearly before the returns statement.

I was missing my first expected time from this setup until I changed the initial offset to -1.

            var tickTock = new[] {
                new DateTimeOffset(2025, 12, 13, 14, 15, 16, 000, TimeSpan.Zero),
                new DateTimeOffset(2025, 12, 13, 14, 15, 17, 100, TimeSpan.Zero),
                new DateTimeOffset(2025, 12, 13, 14, 15, 18, 250, TimeSpan.Zero),
                new DateTimeOffset(2025, 12, 13, 14, 15, 19, 500, TimeSpan.Zero),
                new DateTimeOffset(2025, 12, 13, 14, 15, 20, 800, TimeSpan.Zero),
            };

            var tickTockOffset = -1; // https://github.com/nsubstitute/NSubstitute/issues/689
            var systemClock = Substitute.For<ISystemClock>();

            systemClock.UtcNow
                .Returns(_ => tickTock[tickTockOffset])
                .AndDoes(_ => tickTockOffset += 1); // this happens first

DonFrazier avatar Jul 29 '22 18:07 DonFrazier