NSubstitute icon indicating copy to clipboard operation
NSubstitute copied to clipboard

List other related calls in `Received.InOrder` assertion failure message

Open dtchepak opened this issue 7 years ago • 6 comments

As suggested in https://github.com/nsubstitute/NSubstitute/issues/443#issuecomment-415327557, it could be useful to expand the information presented in the CallSequenceNotFoundException thrown from Received.InOrder.

Possibly something like this:

Expected to receive these calls in order:

   handler.OtherMethod("value1", "value2");
   handler.Execute("value3", "value4");

Actually received matching calls in this order:
    
   handler.OtherMethod("value1", "value2");

Possibly related calls received:

  handler.Execute("other", "example")
  handler.OtherMethod("value1", "another value")
  handler.Execute("another", "example")

*** Note: calls to property getters are not considered part of the query. ***

dtchepak avatar Aug 24 '18 00:08 dtchepak

@dtchepak Be aware that Received.InOrder allows to include multiple subs, like:

handler1.OtherMethod("value1");
handler2.Execute("blah");

So what's is our algorithm for that? What exactly we want to include and considering a fact that we might have a few subs of the same type.. Do we want to work only with the instances and methods used in query? How to handle generics? (The answer is that probably we use only same generic instantiation).

If you write some specs, it might be easier to implement the feature 👨‍💻 🍻

Be aware of #349 - we might cover it here as well, as the problem looks related.

zvirja avatar Aug 30 '18 21:08 zvirja

@zvirja

If you write some specs, it might be easier to implement the feature

Guilty as charged! 😂

If this is in a separate section ("possibly related calls" rather than mixed in with the "actually received") then I wonder if we can get away with listing all calls with the same name to any substitute called in the block?

Basic algorithm I'm thinking of is:

For each substitute called in `Received.InOrder` block:
    get all received calls to substitute
    filter where call name == call name in block

So if we have:

public interface IExample {
    void Sample1<T>(T test);
    int Sample2(string s);
    int Sample3(string s, int i);
}

// actual:
ex.Sample1(42);
ex.Sample3("test", 42);

// test:
Received.InOrder(() => {
    ex.Sample1("test");
    ex.Sample2("test");
});

Then we get:

Possibly related calls received:
    ex.Sample1(42);
    ex.Sample3("test", 42);

This could potentially be a huge list; not sure if that would be a problem?

What do you think of this approach?

dtchepak avatar Aug 31 '18 00:08 dtchepak

@dtchepak Notice, we don't have variable names, therefore we just show the method names:

NSubstitute.Exceptions.CallSequenceNotFoundException : 
Expected to receive these calls in order:

    IBar.Echo("Echo")
    IFoo.CreateBar("Alex")
    IFoo.Test("Bro")

Actually received matching calls in this order:

    IFoo.CreateBar("Alex")
    IFoo.Test("Bro")
    IBar.Echo("Echo")

It might be hard to distinguish on which particular IFoo instance it was invoked. We could prepend substitute ID for that:

Actually received matching calls in this order:

    IFoo.CreateBar("Alex")
    IFoo.Test("Bro")
    IBar.Echo("Echo")

Full list of related calls in the call order:

    [Substitute.IFoo|037bbe83].CreateBar("Alex");
    [Substitute.IFoo|037bbe83].CreateBar("David");
    [Substitute.IFoo|02af7716].CreateBar("David");
....

Basically, I'm fine with your idea and we could start with it. Later we could see how it goes 👍

zvirja avatar Feb 09 '19 22:02 zvirja

@zvirja Good pick up. I like your suggestion, although would drop the Substitute. prefix as I think it should be obvious enough from the context. So just [IFoo|02af7716].Method(args).

We should also add the id to "Actually received ..." as well?

dtchepak avatar Feb 10 '19 22:02 dtchepak

although would drop the Substitute. prefix

Let's see whether there is a nice technical way to solve that. This prefix is a part of a substitute ID which I supposed to use as-is and substringing it would be a quite dirty approach... 😕

We should also add the id to "Actually received ..." as well?

Yep, probably we need to add that in all the lines, as we always inspect the particular substitute only.

In addition, we should come up with a nice title for the last block, where we see all the received, mixed with related calls.

zvirja avatar Feb 11 '19 22:02 zvirja

Any update on this? I find the diagnostic I get to be unhelpful, I'm not sure why it is saying no received calls.

rcdailey avatar Jan 14 '22 03:01 rcdailey