Give mocks names

[pimterry]

Fixes #74. This gives ever mock a unique name initially, of the form Mock<TypeBeingMocked:1234>, where 1234 is a random 4 numbers, to make mocks more easily distinguishable.

This name can be reconfigured, via the new mock.Name property, and the name is passed on to the actual mock object; if there's no explicit implementation of ToString in the hierarchy (so it's falling back to the default object implementation) then it instead takes the name of the mock plus '.Object', e.g. Mock<TypeBeingMocked:1234>.Object. If there is a virtual implementation of .ToString already present in a superclass of the mock, then we proxy that as normal to preserve previous behaviour, returning null as a default value.

[kzu]

This is kinda weird. I would have hoped the return value would be "correct value". Am I missing something?

[pimterry]

Ah, interesting, yes. This is mostly just that I forgot to change the string; it is supposed to return null, because it's a virtual method, and by default Moq should stub out all stubbable methods with no-op implementations that return the relevant default value (false/null/0), and this is one of those.

I'll change it from 'correct value' to make that clearer though, that's definitely a mistake.

[pimterry]

Hmm, I have just discovered that this actually only changes ToString() for mocked interfaces, not mocked classes, because that's how the BaseTypeForInterfaceProxy option works, and I can't see an easy way around it. That's not a disaster I suppose, it's still useful in that case, but it's not as good as I'd like it to be... Any ideas?

[kzu]

If Name cannot be changed for class-based mocks, isn't it a bit less useful
and inconsistent as implemented?

I think it's useful enough if the name is used when rendering the
Mock.ToString only, and nothing more (no overriding of ToString on the
object instance at all). And if people really really wanto to have it also
on the mock.Object, then we see how to make it consistent somehow (i.e.
only override ToString on classes if a name is indeed specified?).

Not sure I like that mocks are automatically named. I'd be more comfortable
if this were opt-in instead.

/kzu

Daniel Cazzulino

On Thu, Dec 19, 2013 at 4:07 PM, Tim Perry notifications@github.com wrote:

Hmm, I have just discovered that this actually only changes ToString() for
mocked interfaces, not mocked classes, because that's how the
BaseTypeForInterfaceProxy option works, and I can't see an easy way around
it. That's not a disaster I suppose, it's still useful in that case, but
it's not as good as I'd like it to be... Any ideas?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/76#issuecomment-30956307
.

[pimterry]

Mock naming on it's own isn't really useful though; afaik you're very rarely looking at error messages that involve only the mocks themselves, it's almost always the resulting mock object that your code is asserting on. Mock naming is really only good for relating the mock to the actual mock object instance anyway, which only works when it has a related name too.

It's worth pointing out that regardless of whether this functionality is added, you're changing the default ToString value anyway; if you mock an object then the castle proxying messes around with it so you end with ToString returning Castle.Proxies.ArrayListProxy (or similar). That seems worse than overriding with a Mock<X:qwe>.Object name, since at least that's meaningful and useful. As is it's exposing internal Moq implementation details that shouldn't be relevant to Moq users, and not providing as much information (i.e. some way to distinguish between instances).

It is definitely inconsistent though as it stands, you're not wrong, and I don't like that either! I'm going to spend a little longer playing with it, and see if I can find a way to get the whole thing working consistently for interfaces and classes. There's then a separate question of whether we include default names at all, or whether it's only if mock.Name is set, but that's a comparatively small implementation detail once the wiring works. Does that sound sensible?

[kzu]

Very good points indeed. Your proposal sounds very sensible indeed :)

Thanks for taking the time to think through these use cases to make this
cool feature great!

/kzu

[pimterry]

Right, sorry that took a while with Christmas and all, but I've now got it finished and working!

With these changes, this currently gives default names to all mock objects, which can be overridden, and now has working implementations that set ToString() to be mock name + '.Object' for both class and interface implementing mock objects.

[kzu]

Any chance you can Fetch+Rebase these commits so they can be automatically merged?

Thanks!

[pimterry]

Ah, sure, done.

[kzu]

Maybe we should call this InterfaceProxy instead? It's a bit more explicit. Not a fan of the "Base" suffix ;)

[pimterry]

All sounds good to me; updated.

[kzu]

I believe this PR broke the Silverlight version, since CSharp is not available as a namespace?

Build FAILED.
"D:\temp\tmp33E3\build.proj" (default target) (1) ->
"D:\temp\tmp33E3\Source.Silverlight\Moq.Silverlight.csproj" (Rebuild target) (4) ->
(CoreCompile target) ->
d:\temp\tmp33E3\Source\Mock.Generic.cs(45,17): error CS0234: The type or namespace name 'CSharp' does not exist in the namespace 'Microsoft' (are you missing an assembly reference?) [D:\temp\tmp33E3\Source.Silverlight\Moq.Silverlight.csproj]

[kzu]

Did you have a chance to look at fixing the Silverlight project?

/kzu

Daniel Cazzulino

On Tue, Jan 7, 2014 at 5:07 AM, Tim Perry notifications@github.com wrote:

All sounds good to me; updated.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/76#issuecomment-31719601
.

[pimterry]

Not right now, although I should do this weekend if you're happy to wait until then.

If not, if you want to fix it yourself I'd suggest just need to wrapping the complaining code in #if !SILVERLIGHT elements, and leave it just using typeof (T).FullName in that case. The names aren't as easily readable for generic types, but they're not disasterous, it's definitely still useful.

[kzu]

yup, agreed. I can wait :)

thanks in advance!

/kzu

Daniel Cazzulino

On Thu, Jan 9, 2014 at 1:47 PM, Tim Perry notifications@github.com wrote:

Not right now, although I should do this weekend if you're happy to wait
until then.

If not, if you want to fix it yourself I'd suggest just need to wrapping
the complaining code in #if !SILVERLIGHT elements, and leave it just
using typeof (T).FullName in that case. The names aren't as easily
readable for generic types, but they're not disasterous, it's definitely
still useful.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/76#issuecomment-31951648
.

[kzu]

I'm afraid @pimterry this refactoring is causing issues in the latest release.

Install it with: Install-Package Moq -Version 4.2.1401.2701

The dynamic proxy cannot access InterfaceProxy, the new base class for interface-based proxies we have. I don't know why, since Moq itself has InternalsVisibleTo DP, but as far as I can see, that's the change that's breaking the latest release.

Plz update issue #98 if you find more about it.

Thanks!