-
-
Notifications
You must be signed in to change notification settings - Fork 803
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
mock.Setup(m => m.A.B.X) should setup only what's minimally required. It shouldn't setup all properties, nor override preexisting setups in A or B. #426
Comments
+1. Issue #191 is preventing me from using |
If it ends up being too risky to break backwards compatibility, it would at least be nice to have a property on a |
I agree, I'm almost sure this change would have to be introduced with a backwards compatibility switch. I'm just not sure what form that switch should take. Newer versions of .NET offer a standardized switch facility via I guess having explicit switches would be better. Your idea of turning |
The only downside I can think of is that enum flags assume that the default case has a value of 0, so the current values would have to be reordered, which would break MockRepository serialization. I'm not sure why anyone would ever want to do that, though... Something like this: [Flags]
public enum MockBehavior
{
Default = 0, //currently 1
[Obsolete("This is the default behavior, and thus should be omitted")]
Loose = 0, //currently 1
Strict = 1, //currently 0
MinimalSetup = 2
} |
@Arithmomaniac - I just released a hotfix update of Moq (because of #500). After giving this some more thought, I opted not to reuse |
Closing this, this problem will be tracked in #643 (together with a couple other related issues). |
A setup such as this:
currently doesn't just set up what is minimally required to set up
X
. That is, it is not equivalent to the following:That's what one might assume would happen, but what actually happens is that Moq also calls
SetupAllProperties
for the traversed objectsA
andB
. Also, ifA
,B
, orX
have previously been set up, it just throws away those and replaces them with the new setups. (This may be simplifying things a bit.) In my opinion, this violates the principle of least astonishment.Also, the current behavior causes problems like #110 and #191.
I'd like to make a case for changing Moq's current behavior, so that a "recursive" setup expression such as the above will only set up what is minimally required to get at
X
and set it up, and it should only set up what is visible in the setup expression. For example, the short setup and the multi-line setup above should be functionally equivalent.I'd welcome any opinions. If you don't have a lot of time to spare, feel free to 👍 (agree) or 👎 (diagree).
The text was updated successfully, but these errors were encountered: