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

Loose mock should also return empty for IReadOnlyList<T> #173

Closed
hickford opened this issue May 21, 2015 · 4 comments
Closed

Loose mock should also return empty for IReadOnlyList<T> #173

hickford opened this issue May 21, 2015 · 4 comments
Assignees
Milestone

Comments

@hickford
Copy link

I think the behaviour described at https://github.com/Moq/moq4/wiki/Quickstart#user-content-customizing-mock-behavior and http://www.nudoq.org/#!/Packages/Moq/Moq/MockBehavior/F/Loose

default behavior is "Loose" mock, which never throws and returns default values or empty arrays, enumerables

Should also apply to the new .NET 4.5 interfaces IReadOnlyList<T> and IReadOnlyCollection<T>

Relevant code: https://github.com/Moq/moq4/blob/92e8ed91820b572a524e7c746d030d673e2295eb/Source/EmptyDefaultValueProvider.cs#L78

@kzu
Copy link
Contributor

kzu commented May 21, 2015

PR link? ;)
On Thu, May 21, 2015 at 6:13 AM Mirth Hickford notifications@github.com
wrote:

The behaviour described at
https://github.com/Moq/moq4/wiki/Quickstart#user-content-customizing-mock-behavior
and http://www.nudoq.org/#!/Packages/Moq/Moq/MockBehavior/F/Loose

default behavior is "Loose" mock, which never throws and returns default
values or empty arrays, enumerables

Should also apply to the new .NET 4.5 interfaces IReadOnlyList and
IReadOnlyCollection

Relevant code:
https://github.com/Moq/moq4/blob/92e8ed91820b572a524e7c746d030d673e2295eb/Source/EmptyDefaultValueProvider.cs#L78

Reply to this email directly or view it on GitHub
#173.

@hickford
Copy link
Author

Oh I'm not sure how to do it without breaking the build for .NET < 4.5

@stakx
Copy link
Contributor

stakx commented Nov 19, 2017

Closed #330 as a duplicate of this issue, but there's a relevant discussion there about ways to make Moq more consistent. Simply changing the (long-standing) current behavior of the various default value settings could be dangerous because it could break a lot of existing user code. It might be better to extend the API so it allows custom, configurable default value providers.

@stakx
Copy link
Contributor

stakx commented Nov 28, 2017

This present issue requests that DefaultValue.Empty should produce an empty collection for the IReadOnlyList<T> type. While it would now be easily possible to fulfill that request, taking it too literally would result in an incomplete change because there are so many more collection types in the System.Collections.* namespaces that EmptyDefaultValueProvider does not currently support.

Neither does it make much sense to just add all collection types because many of them are rarely used (e. g. ArrayList, BitArray, Hashtable, Queue, etc.). All that work to support those will likely not pay off.

What's worse, there are actually some old Moq unit tests that ensure EmptyDefaultValueProvider does not handle certain collection types specially (List<T>!). I don't know why those tests were put in place, but there they are.

Finally, just changing the behaviour or EmptyDefaultValueProvider could potentially break a lot of existing user code.

We could introduce a backward compatibility switch for that (via Mock.Switches), but then there's the question whether it is on or off by default.

I've realised that I really don't want to make all those API decisions of how exactly DefaultValue.Empty behaviour should be extended or changed, how the new behaviour can be tapped into, and I certainly don't want to cause a massive breaking change. I've concluded that it might be best to just leave DefaultValue.Empty precisely how it has worked for the past several years. If folks need a different behaviour, they can now implement their own custom default value providers.

To make this a little easier, I've submitted #536, which introduces a new abstract base class LookupOrFallbackDefaultValueProvider, which is much closer to the empty default value provider than the DefaultValueProvider base class. In fact, it happens to be the new base class for the provider behind DefaultValue.Empty.

See the PR for an example how to create a custom empty default value provider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants