Mocking concrete class that is declared as interface don't use mock any longer

[toha73]

Noticed this when upgrading from Moq 4.7.10 to 4.7.99.

In one of our test we by mistake declared a variable as an interface but mocked against the concrete implementation. After upgrade it started using the concrete method instead of the mocked one. Maybe this was a bug in former version?

The following code works in version 4.7.10 but fails for 4.7.99:

void Main()
{
    ITest test = Mock.Of<Test>();
    Mock.Get(test).Setup(t => t.Try()).Returns("MOCKED-VALUE");
    var result = test.Try();
    Assert.That(result, Is.EqualTo("MOCKED-VALUE"));
}

public interface ITest
{
     string Try();
}

public abstract class Test : ITest
{
    public string Try()
    {
        return "CONCRETE-VALUE";
    }
}

This was easily fixed by mocking against ITest. But was this a planned change in behaviour?

[stakx]

Hi @toha73,

Thanks for reporting. I've run a quick check, the relevant change in Moq's behaviour happened back in version 4.7.58. It is very likely due to the following bugfix in 4.7.58 as mentioned in the changelog:

Fix major "class method vs. interface method" bug introduced by #119 / commit 162a543 (@stakx, #381)

This was a major bug causing tons of issues, the bugfix was therefore very much necessary. The bugfix reverted Moq's behaviour back to how things used to work in previous (pre-bug) versions.

That being said, something strange is going on in your code, I'll report back once I've taken a closer look at it.

[stakx]

@toha73, here's what I found:

ITest test = Mock.Of<Test>();
Mock.Get(test).Setup(t => t.Try())…

I believe the real problem here is that Moq doesn't warn you that you are setting up a sealed method:

public abstract class Test : ITest
{
   public string Try()
   …

Add the virtual keyword to the definition of Test.Try and see what happens then!

Remember, C# makes methods implicitly sealed by default. So unless you explicitly mark a method as virtual (by using either of the abstract, override, or virtual keywords), then it is going to be sealed. Moq cannot intercept sealed methods because the proxy library it is built on (DynamicProxy) doesn't have that capability.

Currently, Moq only complains about setups that involve static or non-virtual methods. Your Test.Try method is neither of those: at the CLR / reflection level, it is both virtual (because it implements ITest.Try) and sealed (because you haven't actively marked it as virtual). Moq doesn't take the latter into account.

I think Moq could & should be more helpful here and complain (read: throw an exception) in situations like yours, when someone is trying to setup a sealed method that it can't actually intercept. Do you agree?

[toha73]

Thank you for the effort. I've already fixed the problem in our code by using Mock.Of<ITest>() instead of Mock.Of<Test>(). It was incorrect test code from the beginning that happened to work because of that bug. I just wanted to notify you of my find if there were some issues with the current revision.

Yes I agree it would be nice if it was possible to throw an exception. Although, not sure if there are cases when you're supposed be able to set up against a method that isn't sealed.

[stakx]

Thanks for the quick answer!

I agree it would be nice if it was possible to throw an exception.

I think I'll prepare a PR for this.

Although, not sure if there are cases when you're supposed be able to set up against a method that isn't sealed.

I am not sure that I understand. You can only set up methods that aren't sealed, for the simple reason that sealed methods cannot be overridden in a derived class (which is what Moq's mocks really are).

[toha73]

My error, I meant "... set up against a method that is sealed".

[stakx]

OK. I'll think there's no valid use case but I'll think about this some more. Thanks again for your valuable feedback! :)

[stakx]

Let's track this as a bug in a separate issue.