-
-
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
Property setups are ignored on mocks instantiated using Mock.Of
#1066
Comments
Hi @stakx, the proposal makes sense. However, I have 2 questions:
|
(2) is a no-go because we should be moving away from different setup types and towards a single unified setup type. (That's what my draft PR for (1) looks about right, although it's possible that there are corner cases that might complicate things. I cannot remember from memory right now but we'd possibly have to consider at least (a) method overrides, (b) indexers, and (c) setups for Re: (a) the possible issue is that Re: (b) and (c), the trouble might be us adding a very non-specific setup with |
(a) (b) the whole block is already wrapped by
|
@lepijohnny – thanks for taking a closer look. Looks about right (except for the potential method override issue, we should perhaps have a test for that, too). Feel free to work on this, if you'd like. |
Let me take a closer look to the ongoing PR about the behavioral setups(I like the idea btw). I will come back with some proposals or dilemmas soon. |
Hello, I believe I am facing this issue. Is there any advice on what to do? I use Are there any alternatives to |
I've been looking into possible ways to fix this. Probably the cleanest approach would be to first refactor Moq such that a single setup can handle several different methods, and then introducing some new setup type This would allow us to fix this issue, as well allow us quite a few bits of code spread everywhere that deals with lazy property stubbing. (I said above that a new setup type would be a no-go. I've since tinkered some more with refactoring setups towards |
This report goes back to a Gitter chat post by @bluechrism on Sep 3 2020:
Moq 4.12.0 introduced a behavioral change that leads to property setups sometimes being ignored on mocks that were created with
Mock.Of<>
(or on mocks on whomSetupAllProperties
was called):Repro code:
Expected behavior:
Both tests should pass, since one would expect that the
SetupGet
setup will be invoked.Actual behavior:
The second test fails.
Likely cause:
#826 made stubbed properties fully lazy. (
Mock.Of<T>
traditionally callsSetupAllProperties
, which stubs all properties.) Setups for stubbed properties are now only added once that property is first queried (or being assigned to). That's the reason why theSetupGet
setup in the second test gets overridden/shadowed by the stubbed property getter.Possible resolution:
If we classify the new behavior (where the second test fails since 4.12.0) as a regression or bug, we could only register stubbed properties' lazy accessor setups if there isn't already a pre-existing setup for that accessor.
/cc @bluechrism @lepijohnny
The text was updated successfully, but these errors were encountered: