NSubstitute icon indicating copy to clipboard operation
NSubstitute copied to clipboard

Identifying substitutes

Open dtchepak opened this issue 14 years ago • 7 comments

Scenario here:

https://gist.github.com/b759fa100814bd04eb11

Could create with a Name or allow overriding ToString(). Perhaps provide default id# for substitutes.

dtchepak avatar Mar 29 '11 04:03 dtchepak

I need it too.

I want to mark my substitutes in the Output window because now I have this:

Expected: not collection containing <Castle.Proxies.IFormProxy>
But was:  < <Castle.Proxies.IFormProxy>, <Castle.Proxies.IFormProxy> >

I want this:

Expected: not collection containing <Bad>
But was:  < <Good>, <Bad> >

Brains avatar Nov 28 '15 10:11 Brains

@dtchepak What if we simply override ToString() to return the incremental number? That will be like Substitute #42, Substitute #421. It should provide the desirable value.

zvirja avatar Feb 28 '18 21:02 zvirja

@zvirja I think it would be useful to add a custom description. And I think we should avoid overriding ToString() for classes that could already have a reasonable implementation. But a reasonable first pass could be auto-numbering for interfaces.

dtchepak avatar Feb 28 '18 22:02 dtchepak

#379 partially addresses this by giving substitutes identifiers based on hashCode, but I would still like the feature of being able to "name" a specific instance of a substitute as per CompleteBrains' comment.

See Zvirja's notes on this PR for some suggestions on implementing this with ProxyIdInterceptor.

dtchepak avatar Mar 22 '18 22:03 dtchepak

@dtchepak I'm not sure we need this feature :confused: Sure, it's interesting from the technical perspective, but I don't see much value in it. The already merged PR allows you to clearly indentify the substitute. The id consists of type name and hash code. It's unlikely that often we will be able to find the better name than resolver for IFooResolver or factManager for IFactManager. As result, the feature might be required really rarely, if never.

The main concern from @CompleteBrains what that it was impossible to recognize the substitute and it was addressed.

I'd suggest to close the issue as is and reopen it later only if there are explicit requests for it. It will allow us to see how it goes with the current way and whether we really need it. WDYT? :wink:

zvirja avatar Mar 23 '18 09:03 zvirja

@zvirja I think there are still some legitimate use cases not covered by the current id scheme.

For mocking in acceptance tests like https://gist.github.com/GraemeF/b759fa100814bd04eb11 I don't think a random identifier will assist in finding the source of the substitute. Instead if we could tag a substitute on creation (something like var estate = Substitute.For<IEstate>(); estate.LabelSubstitute("Estate from DB"); Inject(estate);) then the output could show the substitute label. I think it would be useful to see:

ReceivedCallsException : Expected to receive a call matching:
    Blah(1, <Estate from DB>)
Actually received no matching calls.
Received 2 non-matching calls (non-matching arguments indicated with '*' characters):
    Blah(1, *<Substitute.IEstate|34c247>*)
    Blah(1, *<Estate from cache>*)

I'm happy to wait on another request and a real example (I had lots a long time ago on a previous project, but nothing current) before progressing with this though. :)

dtchepak avatar Mar 23 '18 23:03 dtchepak

This would make tests far more readable, +1 from me.

ryantheleach avatar May 07 '18 07:05 ryantheleach