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

Using object parameter in callback function in place of It.IsAnyType throws exception #1137

Closed
haemmer-dave opened this issue Jan 29, 2021 · 11 comments

Comments

@haemmer-dave
Copy link

In my .Net Core 3.1 app (using Moq 4.16.0) I want to configure a callback on a mocked ILogger object during TestInit(), however, it fails with the following exception when I run the test:

  Message: 
    Initialization method MyNetCoreAppTests.FoobarTests.TestInit threw exception. System.ArgumentException: Invalid callback. Setup on method with parameters (LogLevel, EventId, It.IsAnyType, Exception, Func<It.IsAnyType, Exception, string>) cannot invoke callback with parameters (LogLevel, EventId, object, Exception, Func<object, Exception, string>)..
  Stack Trace: 
    MethodCall.SetCallbackBehavior(Delegate callback) line 174
    SetupPhrase.Callback[T1,T2,T3,T4,T5](Action`5 callback) line 75
    FoobarTests.TestInit() line 21

The code for my minimal working example

namespace MyNetCoreApp
{
    public class Foobar
    {
        public void DoSomething() { }
    }
}

and the test project looks like

using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;
using MyNetCoreApp;
using System;
using System.Collections.Generic;

namespace MyNetCoreAppTests
{
    [TestClass]
    public class FoobarTests
    {
        private Mock<ILogger<Foobar>> loggerMock;

        private readonly List<Tuple<LogLevel, string>> messages = new List<Tuple<LogLevel, string>>();

        [TestInitialize]
        public void TestInit()
        {
            loggerMock = new Mock<ILogger<Foobar>>(MockBehavior.Strict);
            loggerMock.Setup(x => x.Log(It.IsAny<LogLevel>(), 0, It.IsAny<It.IsAnyType>(), It.IsAny<Exception>(), (Func<It.IsAnyType, Exception, string>) It.IsAny<object>()))
                      .Callback<LogLevel, EventId, object, Exception, Func<object, Exception, string>>((level, id, obj, ex, func) =>
                      {
                          messages.Add(Tuple.Create(level, obj.ToString()));
                      });
        }

        [TestMethod]
        public void TestMethod1()
        {
        }
    }
}

According to this comment it should be possible to use object where It.IsAnyType is used. This is confirmed in the corresponding issue #953 (comment). However, I get the exception and even after reading the Quickguide I don't understand what I'm doing wrong. I guess the issue is with the Func<>? I'd appreciate if someone could provide a working example for such a use case.

@stakx
Copy link
Contributor

stakx commented Jan 29, 2021

.Callback<LogLevel, EventId, object, Exception, Func<object, Exception, string>>((level, id, obj, ex, func) =>

I suspect the problem lies not with the third parameter (object), but with the fifth (Func<object, Exception, string>). Note the nested position of where object is used in place of the unknown type.

I may have added support for nested type matchers in #1092, but I was focusing on .Setup—meaning that you should be able to write It.IsAny<Func<It.IsAnyType, Exception,string>>() instead of (Func<It.IsAnyType, Exception, string>) It.IsAny<object>()—but it looks like nested type matchers are not yet taken into account during callback parameter type validation.

⇒ Since you're currently matching the fifth parameter using It.IsAny<object>(), have you tried changing the fifth callback parameter to object, too? That should work.

P.S.: IMHO the signature of that ILogger.Log method (which everyone seems to want to mock, for some reason) simply isn't very amenable to mocking with Moq 4. It may end up being just as easy, but less cryptic, to simply write a mock class manually:

partial class LoggerMock<T> : ILogger<T>
{
    private readonly Action<LogLevel, object> log;
    public Logger(Func<LogLevel, object> log) => this.log = log;
    public void Log<TState>(LogLevel level, EventId _, TState state, Exception __, Func<TState, Exception, string> ___) => this.log(level, state);
}

...
var loggerMock = new LoggerMock<Foobar>((level, obj) => messages.Add(Tuple.Create(level, obj.ToString()));

@haemmer-dave
Copy link
Author

Using object for the fifth callback parameter works. Thanks for the hint. Implementing a mocked logger is not the way that I'd want to go right now.

Regarding the question why this signature of ILogger.Log is mocked so often, I guess it's because that this is the only signature which is not an extension method and when using extension methods I, at least, get the following exception

System.NotSupportedException: Unsupported expression: log => log.Log(It.IsAny<LogLevel>(), (EventId)0, It.IsAny<Exception>(), It.IsAny<string>(), It.IsAny<object[]>())
    Extension methods (here: LoggerExtensions.Log) may not be used in setup / verification expressions..

Since I got a workaround now and this feature was apparently not planned to work with nested type matcher in Callbacks in Moq 4, I'm fine with closing this issue.

@stakx
Copy link
Contributor

stakx commented Feb 1, 2021

Regarding the question why this signature of ILogger.Log is mocked so often [...]

I'm mostly surprised that people need to mock ILogger at all. I'm not very familiar with ASP.NET Core etc. but would have expected that loggers are optional. Mocking a logger would imply that important information ends up in logs, and has to be extracted from there, which I find strange.

I can take a look if it's possible at all to make callback parameter validation less strict in this situation, but I'm not yet sure this is always possible in principle... so I may close this issue soon without taking any further action.

@TsengSR
Copy link

TsengSR commented Jun 17, 2021

@stakx

I'm mostly surprised that people need to mock ILogger at all. I'm not very familiar with ASP.NET Core etc. but would have expected that loggers are optional. Mocking a logger would imply that important information ends up in logs, and has to be extracted from there, which I find strange.

Well, everything is injected via constructor, so its mandatory by default.

Also, the issue lies when one tries to use the Strict mode with auto mock Frameworks which will create a mock for every injected parameter of the system under test.

In Strict mode, if you don't have a Setup, it will fail.

But also there are cases where we want to make sure the logger is called, since logs are important for debugging so some information we want to make sure are there (Exceptions, Errors, Warnings)

@stakx
Copy link
Contributor

stakx commented Jun 17, 2021

Thanks for replying to my question. Makes perfect sense.

everything is injected via constructor, so its mandatory by default

But it's not mandatory to inject a mock logger, right? You could easily implement a do-nothing stub by hand and inject that.

(But sure, creating a throwaway mock object may end up being less work.)

when one tries to use the Strict mode with auto mock Frameworks

where we want to make sure the logger is called, since logs are important for debugging

Granted, I suppose that makes sense.

I wasn't aware that people unit-test their logging, I guess I learnt something new today. :-)

@stakx
Copy link
Contributor

stakx commented Aug 25, 2021

I'm closing this issue, since the immediate problem has been resolved. I've opened a new issue that addresses the one remaining task regarding Moq's callback validation.

@stakx stakx closed this as completed Aug 25, 2021
@lorddev
Copy link

lorddev commented Jun 23, 2022

partial class LoggerMock<T> : ILogger<T>
{
    private readonly Action<LogLevel, object> log;
    public Logger(Func<LogLevel, object> log) => this.log = log;
    public void Log<TState>(LogLevel level, EventId _, TState state, Exception __, Func<TState, Exception, string> ___) => this.log(level, state);
}

This doesn't compile. Is there another example?

@PForever
Copy link

PForever commented Sep 4, 2022

You can't use Func<object, Exception, string> to genericize a Func<T, Exception, string> argument, because in the case of Func's input parameters it's not you generalizing, it's client forced to generalize by you. In other words, you're telling the caller to give you a method that can handle ANY input type, but he only knows how to handle type T:

class Client
{
    void Foo(Func<T, Func<int, bool>> callback)
    {
        callback(IsZero);
    }
    bool IsZero(int value) => value == 0;
    // the client knows how to compare 0 with int, but has no idea how to compare 0 with any other type,
    // so if we suddenly wanted Func<int, object>>, he would be confused
}

If you're interested, this comes down to the notion of type covariance and contravariance in C#.

The way out in a particular case is this: generalize not the generic type, but the argument itself: that is, ask the client for SOME delegates, and decide for yourself what type it should be:

Action<LogLevel, EventId, object, Exception?, Delegate> invokeCallback = (l, _, s, ex, f) =>
{
    var message = f.DynamicInvoke(s, ex) as string;
    _messages.Add(message);
};
mock.Setup(s => s.Log(It.IsAny<LogLevel>(),
                      It.IsAny<EventId>(),
                      It.IsAny<It.IsAnyType>(),
                      It.IsAny<Exception?>(),
                      It.IsAny<Func<It.IsAnyType, Exception?, string>>()))
                 .Callback(invokeCallback);

@CumpsD
Copy link

CumpsD commented Mar 13, 2023

I am struggling with something similar, but with more generics :)

I have a simple interface:

public interface IQuery {}

public interface IQuery<TResult> : IQuery { }

public interface IQueryDispatcher
{
    Task<Result<TResult>> ExecuteAsync<TQuery, TResult>(
        TQuery query,
        CancellationToken cancellationToken)
        where TQuery : IQuery<TResult>;
}

An example call of this:

var response = await QueryDispatcher
    .ExecuteAsync<ListNewsletterSubscriptionsQuery, IEnumerable<ListNewsletterSubscriptionsResponse>>(
        new ListNewsletterSubscriptionsQuery(emailClaim.Value), ct);

But I can't figure out how to define the Setup to catch all possible invocations. I've tried this, but it never gets hit:

queryDispatcher
    .Setup(x => x.ExecuteAsync<IQuery<It.IsAnyType>, It.IsAnyType>(
        It.IsAny<IQuery<It.IsAnyType>>(),
        It.IsAny<CancellationToken>()))
    .Callback((IQuery query, CancellationToken _) => queryCapture((TQuery)query))

I can't pass It.IsAnyType as the first generic parameter in the setup, since the code to be tested requires it to be IQuery of the second generic parameter.

I've also tried to be adventerous with dynamic but no luck there either:

queryDispatcher
    .Setup(x => x.ExecuteAsync<IQuery<dynamic>, dynamic>(
        It.IsAny<IQuery<dynamic>>(),
        It.IsAny<CancellationToken>()))
    .Callback((dynamic query, CancellationToken _) => queryCapture((TQuery)query))

Is there a way to catch this kind of structure?

@sbloomfieldtea
Copy link

For anyone stumbling on this and to answer @lorddev here's a version of the LoggerMock that compiles. I added the Exception because I needed to validate that exceptions were indeed being logged (we use Seq a lot for troubleshooting problems). Many thanks to @stakx for the suggestion.

partial class LoggerMock<T> : ILogger<T>
    {        
        private readonly Action<LogLevel, string, Exception> _log;

        public LoggerMock(Action<LogLevel, string, Exception> log) => this._log = log;

        public IDisposable BeginScope<TState>(TState state) where TState : notnull
        {
            return state as IDisposable;
        }

        public bool IsEnabled(LogLevel logLevel)
        {
            return true;
        }

        public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter) => this._log(logLevel, state.ToString(), exception);
    }

My usage:

var loggedMessages = new List<(LogLevel LogLevel, string Message, Exception Exception)>();
var loggerMock = new LoggerMock<MyClass>((LogLevel logLevel, string message, Exception exception) => loggedMessages.Add((logLevel, message, exception)));
//
//
var loggedErrors = loggedMessages.Where(m => m.LogLevel == LogLevel.Error);
Assert.AreEqual(1, loggedErrors.Count());
...etc

@vova-lantsov-dev
Copy link

I needed to mock an ILoggerFactory instance to write the logs into ITestOutputHelper.

I tried mocking the ILogger interface but couldn't, so I created a custom logger class instead.

Here's the code:

private sealed class TestOutputHelperLogger : ILogger, IDisposable
{
	private readonly string _name;
	private readonly ITestOutputHelper _testOutputHelper;

	private static readonly ConsoleFormatter Formatter = CreateSimpleConsoleFormatter();

	private static readonly IExternalScopeProvider ExternalScopeProvider =
		new Mock<IExternalScopeProvider>().Object;

	public TestOutputHelperLogger(string name, ITestOutputHelper testOutputHelper)
	{
		_name = name;
		_testOutputHelper = testOutputHelper;
	}

	public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception,
		Func<TState, Exception, string> formatter)
	{
		var logEntry = new LogEntry<TState>(logLevel, _name, eventId, state, exception, formatter);
		var stringWriter = new StringWriter();

		Formatter.Write(logEntry, ExternalScopeProvider, stringWriter);

		_testOutputHelper.WriteLine(stringWriter.ToString());
	}

	public bool IsEnabled(LogLevel logLevel) => true;

	public IDisposable BeginScope<TState>(TState state) where TState : notnull
	{
		return this;
	}

	public void Dispose()
	{
	}

	private static ConsoleFormatter CreateSimpleConsoleFormatter()
	{
		const string simpleConsoleFormatterTypeName = "SimpleConsoleFormatter";
		const string formatterOptionsMonitorTypeName = "FormatterOptionsMonitor";

		var formatterAssemblyTypes = typeof(ConsoleFormatter).Assembly.GetTypes();

		Type simpleConsoleFormatterType = formatterAssemblyTypes
			.FirstOrDefault(t => t.Name == simpleConsoleFormatterTypeName);
		Type formatterOptionsMonitorType = formatterAssemblyTypes
			.FirstOrDefault(t => t.Name.StartsWith(formatterOptionsMonitorTypeName));

		if (simpleConsoleFormatterType == null)
			throw new Exception(
				"Internal class SimpleConsoleFormatter was not found. Probably, its name changed after the migration to a newer .NET version");
		if (formatterOptionsMonitorType == null)
			throw new Exception(
				"Internal class FormatterOptionsMonitor<T> was not found. Probably, its name changed after the migration to a newer .NET version");

		var monitorOfSimpleConsoleFormatterType =
			formatterOptionsMonitorType.MakeGenericType(typeof(SimpleConsoleFormatterOptions));
		var monitorOfSimpleConsoleFormatter = Activator.CreateInstance(monitorOfSimpleConsoleFormatterType,
			new SimpleConsoleFormatterOptions());

		return (ConsoleFormatter)Activator.CreateInstance(simpleConsoleFormatterType,
			monitorOfSimpleConsoleFormatter);
	}
}

This logger uses existing SimpleConsoleFormatter, which prepares the output in console-based format.

Mock<ILoggerFactory> loggerFactoryMock = new Mock<ILoggerFactory>();
loggerFactoryMock.Setup(f => f.CreateLogger(It.IsAny<string>()))
	.Returns((string name) => new TestOutputHelperLogger(name, _testOutputHelper));

services.AddSingleton(loggerFactoryMock.Object);
services.AddSingleton(typeof(ILogger<>), typeof(Logger<>));

@devlooped devlooped locked and limited conversation to collaborators Sep 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants