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

When mocking a class, why are non-virtual methods hidden in the generated proxy? #212

Closed
diaconesq opened this issue Oct 31, 2015 · 5 comments

Comments

@diaconesq
Copy link

diaconesq commented Oct 31, 2015

I have noticed a change between Moq 4.1.1 and 4.2.1.

In Moq 4.1.1, when creating a new Mock<ActualClass>(), the generated proxy type would not do anything special for non-virtual methods.

In Moq 4.2.1, the (public) non-virtual methods are hidden by new implementations (with the new keyword) in the generated proxy.

Why is that?
I still can't call .Setup(...) or .Verify(...) on non-virtual methods. (throws a 'NotSupported' exception, like it always did. ) It looks as if Moq is preparing things in order to be able to intercept those methods as well.

I'm asking because I think this violates the principle of least astonishment.
(I would have expected that calling the non-virtual method on the mock would call the actual implementation in the original class, regardless of specifying CallBase = true or not)

I have attached below a unit test that was broken by upgrading from Moq 4.1.1 to 4.2.1.

using System.Linq;
using Moq;
using NUnit.Framework;

public interface ILog
{
    void Log(string s);
    void LogFormat(string fmt, params object[] args);
}

public class MockableLog: ILog
{
    public void Log(string s)
    {
        WasLogged(s);
    }

    public void LogFormat(string fmt, params object[] args)
    {
        WasLogged(string.Format(fmt, args));
    }

    public virtual void WasLogged(string message)
    {
    }
}

public class UsesLog
{
    private readonly ILog log;

    public UsesLog(ILog log)
    {
        this.log = log;
    }

    public void LogSomething()
    {
        log.Log("test 1");

        //changing to this should not break the test:
        //log.LogFormat("test {0}", 1);
    }
}

[TestFixture]
public class UsesLogTest
{
    [Test]
    public void CheckLogWasCalledTwice()
    {
        //this used to work with Moq 4.1, not with 4.2
        //var fakeLog = new Mock<MockableLog>();

        //this works
        var fakeLog = new Mock<MockableLog>() { CallBase = true };
        var sut = new UsesLog(fakeLog.Object);


        var methods = fakeLog.Object.GetType().GetMethods()
            .Select(mi => new {mi.DeclaringType, mi.Name, mi.IsHideBySig})
            .OrderBy(mi => mi.Name).ThenBy(mi => mi.DeclaringType.Name)
            .ToList();
        sut.LogSomething();

        //fakeLog.Verify(log => log.Log(It.IsAny<string>()));

        //fakeLog.Verify(log => log.WasLogged(It.IsAny<string>()));
    }
}
@kzu
Copy link
Member

kzu commented Oct 31, 2015

Looks like an implementation detail that changed in the Castle.DynamicProxy
generator. You'll most likely see the same behavior in all the other
mocking libraries too.

On Sat, Oct 31, 2015, 6:28 AM Cristian Diaconescu notifications@github.com
wrote:

I have noticed a change between Moq 4.1.1 and 4.2.1.

In Moq 4.1.1, when creating a new Mock(), the generated
proxy type would not do anything special for non-virtual methods.

In Moq 4.2.1, the (public) non-virtual methods are hidden (with the new
keyword) by the generated proxy.

Why is that?
I still can't call .Setup(...) or .Verify(...) on non-virtual methods.
(throws a 'NotSupported' exception, like it always did. )

I'm asking because I think this violates the principle of least
astonishment.

I have attached below a unit test that was broken by upgrading from Moq
4.1.1 to 4.2.1.

using System.Linq;
using Moq;
using NUnit.Framework;

namespace TestMock
{
public interface ILog
{
void Log(string s);

void LogFormat(string fmt, params object[] args);

}

public class MockableLog: ILog
{
public void Log(string s)
{
WasLogged(s);
}

public void LogFormat(string fmt, params object[] args)
{
    WasLogged(string.Format(fmt, args));
}

public virtual void WasLogged(string message)
{
}

}

public class UsesLog
{
private readonly ILog log;

public UsesLog(ILog log)
{
    this.log = log;
}

public void LogSomething()
{
    log.Log("test 1");

    //changing to this should not break the test:
    //log.LogFormat("test {0}", 1);
}

}

[TestFixture]
public class UsesLogTest
{
[Test]
public void CheckLogWasCalledTwice()
{
//this used to work with Moq 4.1, not with 4.2
//var fakeLog = new Mock();

    //this works
    var fakeLog = new Mock<MockableLog>() { CallBase = true };
    var sut = new UsesLog(fakeLog.Object);


    var methods = fakeLog.Object.GetType().GetMethods()
        .Select(mi => new {mi.DeclaringType, mi.Name, mi.IsHideBySig})
        .OrderBy(mi => mi.Name).ThenBy(mi => mi.DeclaringType.Name)
        .ToList();
    sut.LogSomething();

    //fakeLog.Verify(log => log.Log(It.IsAny<string>()));

    //fakeLog.Verify(log => log.WasLogged(It.IsAny<string>()));
}

}

}


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

@dougbu
Copy link

dougbu commented Aug 19, 2016

Still seeing this in Moq 4.5.12.

Due to the #129 fix, it is possible to Setup() non-virtual void implementations of interface members e.g. the following works

Mock<Stream> mock = new Mock<Stream>(MockBehavior.Strict);
mock.As<IDisposable>().Setup(s => s.Dispose());
mock.Protected().Setup("Dispose", true).Callback(() => innerStreamDisposed = true);

// Do something, perhaps w/ a wrapper, that calls `mock.Object.Dispose()`.

But approaches like the above don't work at all if the interface member returns a value. E.g. the System.Web.Http.ExceptionHandler class implements IExceptionHandler.HandleAsync() explicitly and pretty much can't be mocked w/ MockBehavior.Strict:

public abstract class ExceptionHandler : IExceptionHandler
{
    Task IExceptionHandler.HandleAsync(ExceptionHandlerContext context, CancellationToken cancellationToken)
    {
        // ...
        return HandleAsync(context, cancellationToken);
    }

    public virtual Task HandleAsync(ExceptionHandlerContext context, CancellationToken cancellationToken)
    {
        // ...
        return TaskHelpers.Completed();
    }
}

Mock<ExceptionHandler> mock = new Mock<ExceptionHandler>(MockBehavior.Strict);
mock
    .As<IExceptionHandler>()
    .Setup(h => h.HandleAsync(It.IsAny<ExceptionHandlerContext>(), It.IsAny<CancellationToken>()));

throws a MockException because the Setup() doesn't provide a value.

@dougbu
Copy link

dougbu commented Aug 19, 2016

Hit Comment too soon. Added another sentence or so.

@dougbu
Copy link

dougbu commented Aug 19, 2016

General question: Why is it correct for MockBehavior.Strict to block calls that cannot be setup?

dougbu added a commit to aspnet/AspNetWebStack that referenced this issue Aug 25, 2016
- work around devlooped/moq#129 and devlooped/moq#212; Moq creates proxies for all `interface` methods
 - e.g. set up `IDisposable.Dispose()` calls
 - related bug devlooped/moq#212 means we can't use `MockBehavior.Strict` in some cases
  - especially when method returns a value and therefore can't be set up to call base
  - `CallBase = true` is often required in these cases
- work around devlooped/moq#149; `DefaultValue.Mock` restrictions
 - where necessary, explicitly set up members instead
- work around odd failures in `JsonResultTest` using `SetupSet()` on deep properties
 - use `MockBehavior.Strict` in `NullContentIsNotOutput()` to confirm `Write()` is not called
- handle less-predictable proxy type names
dougbu added a commit to aspnet/AspNetWebStack that referenced this issue Aug 26, 2016
- #11 (3 of 4)
- work around devlooped/moq#129 and devlooped/moq#212; Moq creates proxies for all `interface` methods
 - e.g. set up `IDisposable.Dispose()` calls
 - related bug devlooped/moq#212 means we can't use `MockBehavior.Strict` in some cases
  - especially when method returns a value and therefore can't be set up to call base
  - `CallBase = true` is often required in these cases
- work around devlooped/moq#149; `DefaultValue.Mock` restrictions
 - where necessary, explicitly set members up instead
- work around odd failures in `JsonResultTest` using `SetupSet()` on deep properties
 - use `MockBehavior.Strict` in `NullContentIsNotOutput()` to confirm `Write()` is not called
- handle less-predictable proxy type names
@stakx
Copy link
Contributor

stakx commented Jun 23, 2017

@diaconesq: I couldn't get your test to work. It either always failed or always succeeded with pretty much any version of Moq. So I changed it to the following. I think it is equivalent to what you originally wanted to demonstrate:

public interface ILog
{
    void Log(string s);
}

public class MockableLog : ILog
{
    public void Log(string s) => WasLogged(s);
    public virtual void WasLogged(string message) { }
}

public class UsesLog
{
    private readonly ILog log;

    public UsesLog(ILog log)
    {
        this.log = log;
    }

    public void LogSomething() => log.Log("test 1");
}

[Fact]
public void Test()
{
    var mock = new Mock<MockableLog>();
    var sut = new UsesLog(mock.Object);
    sut.LogSomething();
    mock.Verify(log => log.WasLogged(It.IsAny<string>()));
}

This test breaks when upgrading from 4.2.1402.2112 (where it passes) to 4.2.1408.619 (where it fails). This has been fixed by #381, so you should be fine once you upgrade to Moq 4.7.63.

@dougbu: If you're still interested in an answer to your question above, could you please ask your question as a separate issue so we can track it independently of the originally reported issue? Thanks! :)

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

4 participants