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

Heisenberg's virtual property set in ctor with MockBehavior.Strict and VerifyAll #456

Closed
dfederm opened this issue Sep 24, 2017 · 4 comments
Labels

Comments

@dfederm
Copy link

dfederm commented Sep 24, 2017

I'm trying to mock AspNetCore's UserManager<T> for a unit test and it seems to be exhibiting some strange behavior.

It sets the virtual Logger property in its ctor (isn't this not recommended by MSFT?). When neglecting to mock the Logger setter, I get the following error when constructing the object, as expected:

Message: Moq.MockException : UserManager`1.Logger = null invocation failed with mock behavior Strict.
All invocations on the mock must have a corresponding setup.

However, if I mock the setter, I get an error when calling VerifyAll:

Message: Moq.MockVerificationException : The following setups were not matched:
UserManager`1 _ => _.Logger = (ILogger)It.IsAny<ILogger`1>()

So when I mock it, it says it's never called, and if I don't mock it, it gets called!

Code:

[Fact]
public void DamnedIfIDo()
{
    var mockUserStore = new Mock<IUserStore<ApplicationUser>>(MockBehavior.Strict);
    var mockUserManager = new Mock<UserManager<ApplicationUser>>(
        MockBehavior.Strict,
        mockUserStore.Object,
        null,
        null,
        null,
        null,
        null,
        null,
        null,
        null);
    mockUserManager.SetupSet(_ => _.Logger = It.IsAny<ILogger<UserManager<ApplicationUser>>>());
    var userManager = mockUserManager.Object;

    // Throws because the SetSetup wasn't matched
    mockUserManager.VerifyAll();
}

[Fact]
public void DamnedIfIDont()
{
    var mockUserStore = new Mock<IUserStore<ApplicationUser>>(MockBehavior.Strict);
    var mockUserManager = new Mock<UserManager<ApplicationUser>>(
        MockBehavior.Strict,
        mockUserStore.Object,
        null,
        null,
        null,
        null,
        null,
        null,
        null,
        null);

    // Throws because I didn't mock Logger
    var userManager = mockUserManager.Object;
}
@dfederm dfederm changed the title Heisenberg's virtual property set in setter with MockBehavior.Strict and VerifyAll Heisenberg's virtual property set in ctor with MockBehavior.Strict and VerifyAll Sep 24, 2017
@stakx
Copy link
Contributor

stakx commented Sep 24, 2017

For further analysis, here's a standalone repro (independent of ASP.NET Core) for the same issue:

public interface ILogger { }

public class UserManager
{
    public UserManager(ILogger logger)
    {
        this.Logger = logger;
    }

    public virtual ILogger Logger { get; set; }
}

[Fact]
public void DamnedIfIDo()
{
    var mockUserManager = new Mock<UserManager>(MockBehavior.Strict, null);
    mockUserManager.SetupSet(um => um.Logger = null);
    var userManager = mockUserManager.Object;
    mockUserManager.VerifyAll();
}

@stakx
Copy link
Contributor

stakx commented Sep 24, 2017

Thanks for reporting this issue.

This is something of a current limitation / bug in Moq, and it's caused by good old FluentMockContext, an internal infrastructure component (see this issue).

SetupSet currently has to trigger (here) an early instantiation of the mock object before the setup is completed. This means that the UserManager constructor runs and sets the Logger property before the mock fully knows about the setup. The setter invocation therefore doesn't get recorded. Then later, the setup is done and your mock is now awaiting your code to set Logger -- only that's already happened and won't happen again, therefore the MockVerificationException.

Fixing this bug (and many others of a similar kind, see the issue list in the issue referenced above) is going to be hard work and can't be done on the very short term.

@dfederm, for the moment, if you, for whatever reason, cannot change mockUserManager to be a loose mock and need to keep it strict, here's one possible workaround:

var mockUserStore = new Mock<IUserStore<ApplicationUser>>(MockBehavior.Strict);
var mockUserManager = new Mock<UserManager<ApplicationUser>>(
    MockBehavior.Strict,);
mockUserManager.SetupProperty(_ => _.Logger); // instead of SetupSet; allows the ctor to set Logger
var userManager = mockUserManager.Object;
userManager.Logger = userManager.Logger; // pointless, but satisfies the above setup
mockUserManager.VerifyAll();

@dfederm
Copy link
Author

dfederm commented Sep 24, 2017

Thanks! That workaround seems a little better than giving up either Strict or VerifyAll

@stakx
Copy link
Contributor

stakx commented Jul 13, 2018

Closing this, this problem will be tracked in #414 (together with a couple other related issues).

@stakx stakx closed this as completed Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants