NSubstitute icon indicating copy to clipboard operation
NSubstitute copied to clipboard

Make ReceivedCalls() API "an official" and document it

Open simonvane opened this issue 5 years ago • 10 comments

Is your feature request related to a problem? Please describe. I would like to be able to assert that no methods were called in a substitute rather than any specific method.

Describe the solution you'd like Id like to be able to do the following: theSubstitute.DidNotReceive(); or perhaps theSubstitute.DidNotReceiveAny();

rather than having to describe each method that shouldn't be called.

Describe alternatives you've considered At the moment my alternatives are:

  1. The tests to have to know a lot more about the internals of the SUT than I'd like and specify particular methods I don't want called.
  2. Specify DidNotReceieve for every method in the substitute.

simonvane avatar Mar 18 '19 11:03 simonvane

@simonvane Please take a look at the substitute.ReceivedCalls(), it should do a trick. Just assert that collection is empty - that's it 😉

zvirja avatar Mar 18 '19 12:03 zvirja

Thanks for your quick response @zvirja , much appreciated. That method does the job thanks.

This was not easy to discover so I wonder:

  1. Would it be worth having the DidNotReceiveAny method that does something like substitute.ReceivedCalls().Any() == false?
  2. Could something be added to the documentation in here https://nsubstitute.github.io/help/received-calls/ about ReceivedCalls?

NSubstitute.Core.Calls does not expose any public information about the calls that were made so it makes it difficult to add useful information to my asserts.

Is there any way of getting more information about the calls?

simonvane avatar Mar 18 '19 12:03 simonvane

Would it be worth having the DidNotReceiveAny method that does something like substitute.ReceivedCalls().Any() == false

To me it's very rare case that you want to have assertions like this, so I'm not really sure the library should come with it. But it's just my opinion 😉

Could something be added to the documentation in here https://nsubstitute.github.io/help/received-calls/ about ReceivedCalls?

@dtchepak Could you please elaborate on that?

NSubstitute.Core.Calls does not expose any public information about the calls that were made so it makes it difficult to add useful information to my asserts. Is there any way of getting more information about the calls?

Could you please elaborate more on which particular information you are looking for? I'm afraid it's the only API we have to obtain received call information which comes to my mind...

zvirja avatar Mar 19 '19 21:03 zvirja

@zvirja ReceivedCalls was never part of the officially published API, as I wasn't sure if we were going to do a nice query API to support getting information about subsets of the received calls. ReceivedCalls was sort of a hack in the meantime in case people didn't mind diving into some of the internal representations (and wading through MethodInfos and similar :)).

Given the 10 years or so it's been on the todo list, maybe we should just bless the ReceivedCalls method with "official" status and add it to the docs? 😂

I'd prefer to avoid adding DidNotReceiveAny as it would further add to our bloating of intellisense on Object, and it is easy enough for people to add to individual projects if they need it. (or just assert on ReceivedCalls as you mentioned.)

dtchepak avatar Mar 20 '19 00:03 dtchepak

@dtchepak After giving it a more precise look, I don't want to make the ReceivedCalls extension method "an official" API for now 😟 The problem is that the ICall interface (return type) is too internal. It provides API which could rewrite historical info about call (call.GetArguments()[xxx] = 'xxx') or just do crazy things (xxx.TryCallBase()). I think we should make a wrapper which will return limited amount of information only and in read-only manner.

I suggest to change the shape in this feature and document it only afterwards 😉

zvirja avatar Mar 24 '19 20:03 zvirja

My use case for a method to check that no calls were received are either "no data" cases or exception cases i.e. if there is not data found or and exception thrown, I want to check that nothing has been called on certain substitutes.

I do this kind of checking far less than other substitute use cases but I wouldn't go as far as to say rare.

I have found in code reviews a misapprehension that substitute.DidNotReceive(); ensures nothing was called on the substitute so including a DidNotReceiveAnyCalls method might avoid that issue. (I do realise that this misunderstanding also says something about how the tests were written in that the developer had not tested that the test failed in the correct way when conditions weren't met.)

I was mistaken about what is returned from (ICall) ReceivedCalls; it has all the information required to return a good assertion message.

It would be great if ReceivedCalls was documented. It would be even better in my opinion if a DidNotReceiveAnyCalls method was added.

Here is an unfinished example of what I have in mind and have had to write myself:

public static void DidNotReceiveAnyCalls<T>(this T substitute)
    where T : class
{
    if (substitute.ReceivedCalls().Any())
    {
        var methodsCalled = string.Join(", ", substitute.ReceivedCalls().Select(c =>
        {
            var m = c.GetMethodInfo();
            return m == null ? string.Empty : $"{m.DeclaringType.Name}.{m.Name}";
        }));
        throw new ReceivedCallsException($"Expected to receive no calls but the following were called: {methodsCalled}");
    }
}

simonvane avatar Apr 06 '19 07:04 simonvane

It would be great if ReceivedCalls was documented.

Yes it would. There's no docs, and it's intended usage isn't obvious even with the description on intellisense (on v4.0.0 at least).

ZodmanPerth avatar Jan 13 '21 06:01 ZodmanPerth

Sorry I don't have anything concrete to add, but I suppose this is still the way to do this in 2023? Migrating from Moq I was missing this feature and I only discovered this feature actually exists thanks to a StackOverflow answer (I read all the documentation pages for NSubstitute except "How NSubstitute works").

hhyyrylainen avatar Sep 03 '23 18:09 hhyyrylainen