Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Identifying substitutes #44

Open
dtchepak opened this issue Mar 29, 2011 · 7 comments
Open

Identifying substitutes #44

dtchepak opened this issue Mar 29, 2011 · 7 comments
Labels
feature-request Request for a new NSubstitute feature

Comments

@dtchepak
Copy link
Member

Scenario here:

https://gist.github.com/b759fa100814bd04eb11

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

@Brains
Copy link

Brains commented Nov 28, 2015

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> >

@dtchepak dtchepak added the feature-request Request for a new NSubstitute feature label Oct 7, 2017
@zvirja
Copy link
Contributor

zvirja commented Feb 28, 2018

@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.

@dtchepak
Copy link
Member Author

@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
Copy link
Member Author

#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.

@zvirja
Copy link
Contributor

zvirja commented Mar 23, 2018

@dtchepak I'm not sure we need this feature 😕 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? 😉

@dtchepak
Copy link
Member Author

@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. :)

@ryantheleach
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for a new NSubstitute feature
Projects
None yet
Development

No branches or pull requests

4 participants