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

Feature request: enable verification of extension-methods calls #1045

Closed
weitzhandler opened this issue Aug 3, 2020 · 3 comments
Closed

Comments

@weitzhandler
Copy link
Contributor

weitzhandler commented Aug 3, 2020

Hi,

I often come across mocks that expose complex methods with multiple complex parameters of lambdas etc.
Those methods are provided with companion methods that enable passing just one or two primitive-type arguments delegate them to the actual complex method.

Let's use the Microsoft.Extensions.Logging.ILogger.Log method as an example. It has the following signature:

public void Log<TState>(
  LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter);

Pretty complex, but in essence you would almost never call it directly and would instead call the various extension methods in LoggerExtensions.

I wonder how hard it could be enabling verification through extension methods.

One way could be (and I'm sure there would be other, better, ideas), when calling LogWarning (LogWarning (this ILogger logger, string message, params object[] args);, could we internally prepare a verification expectation by recording what's being called downstream on a similar dummy mock, so that we could then verify it was called on the actual mock?

That would enable us to write

loggerMock
  .Verify(logger => logger.LogWarning("Error occurred"), Times.Once);

which is what we actually want to verify, instead of the one that's so tedious and taking so long to figure out and make it work

public static void VerifyLog<TCategoryName>(this Mock<ILogger<TCategoryName>> loggerMock, LogLevel logLevel, string expectedMessage)
{
  loggerMock.Verify(logger =>
    logger.Log(
      logLevel,
      It.IsAny<EventId>(),
      It.Is<It.IsAnyType>((state, type) => state.ToString() == expectedMessage),
      It.IsAny<Exception>(),
      It.Is<Func<It.IsAnyType, Exception, string>>((func, type) => true)), //IsAny doesn't work
    Times.Once);
}

I'm willing to contribute.

@stakx
Copy link
Contributor

stakx commented Aug 3, 2020

I wonder how hard it could be enabling verification through extension methods.

If you want to do this the right way, it's basically impossible, unless you use the CLR's unmanaged Profiling API; but then you sacrifice portability (think e.g. Mono) so that's currently a no-go.

could we internally prepare a verification expectation by recording what's being called downstream on a similar dummy mock,

That's very similar to how expression reconstruction works for SetupSet/VerifySet: those are given a delegate, which gets executed on a dummy mock. The virtual, overridable methods invoked on the dummy mock are recorded, and from that a LINQ expression tree gets reconstructed.

That approach has several downsides, basically it only works correctly under very specific conditions. If we used that approach to support extension methods, we'd have to assume that extension methods deterministically call exactly one virtual, overridable method on the this argument. If the extension method calls none, or more than 1 such method, or if it calls different methods depending on its input, you're going to see some weird behavior.

IMHO it's best not to go there... this will complicate Moq internals quite a bit, make Moq's LINQ expression tree handling less precise, and it'll be a fragile feature at best. As such is begging for support questions and false bug reports.

@weitzhandler
Copy link
Contributor Author

Your kind and comprehensive answer makes a lot of sense.
Thanks for taking the time to write it.

@weitzhandler
Copy link
Contributor Author

weitzhandler commented Aug 4, 2020

Why doesn't this work:

It.IsAny<Func<It.IsAnyType, Exception, string>>()

Whereas this one does?

It.Is<Func<It.IsAnyType, Exception, string>>((func, type) => true))

Moq.MockException :
Expected invocation on the mock once, but was 0 times: logger => logger.Log<It.IsAnyType>(LogLevel.Error, It.IsAny(), It.Is<It.IsAnyType>((state, type) => state.ToString() == "Attempted to initialize tenant with subdomain 'russel', which does not exist, is not initialized or inactive."), It.IsAny(), It.IsAny<Func<It.IsAnyType, Exception, string>>())
Performed invocations:
Mock<ILogger:4> (logger):
ILogger.Log(LogLevel.Error, 0, Attempted to initialize tenant with subdomain 'russel', which does not exist, is not initialized or inactive., System.InvalidOperationException: Attempted to initialize tenant with subdomain 'russel', which does not exist, is not initialized or inactive.

at MyProject.Api.Services.Extensions.LoggerExtensions.LogAndThrow(ILogger logger, LogLevel logLevel, Exception exception, String message) in D:\Users\Shimmy\Source\Repos\MyProject\MyProject.Api\Services\Extensions\LoggerExtensions.cs:line 24
at MyProject.Api.Services.Extensions.LoggerExtensions.LogAndThrow(ILogger logger, LogLevel logLevel, Exception exception) in D:\Users\Shimmy\Source\Repos\MyProject\MyProject.Api\Services\Extensions\LoggerExtensions.cs:line 18
at MyProject.Api.Services.Extensions.LoggerExtensions.LogAndThrow(ILogger logger, LogLevel logLevel, String message) in D:\Users\Shimmy\Source\Repos\MyProject\MyProject.Api\Services\Extensions\LoggerExtensions.cs:line 15
at MyProject.Api.Services.Extensions.LoggerExtensions.LogAndThrow(ILogger logger, String message) in D:\Users\Shimmy\Source\Repos\MyProject\MyProject.Api\Services\Extensions\LoggerExtensions.cs:line 12
at MyProject.Api.Services.TenantStore.EnsureTenantInitializedAsync(String subdomain) in D:\Users\Shimmy\Source\Repos\MyProject\MyProject.Api\Services\Storage\TenantStore.cs:line 73
at Xunit.Record.ExceptionAsync(Func`1 testCode) in C:\Dev\xunit\xunit\src\xunit.core\Record.cs:line 76, Func<FormattedLogValues, Exception, string>)
Stack Trace:
Mock.Verify(Mock mock, LambdaExpression expression, Times times, String failMessage)
Mock`1.Verify(Expression`1 expression, Times times)
Mock`1.Verify(Expression`1 expression, Func`1 times)
LoggerMockExtensions.VerifyLog[TCategoryName](Mock`1 loggerMock, LogLevel logLevel, String expectedMessage) line 11
TenantStoreTests.Should_log_error_when_initializing_tenant_if_not_exists() line 129
--- End of stack trace from previous location ---

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

2 participants