Log[Severity] Extension Methods Make Unit Testing Logging Behavior Difficult

[ardalis]

Using extension methods rather than interface methods on ILogger makes it difficult to mock an ILogger effectively, which in turn makes unit testing logging behavior in classes that perform logging difficult.

Consider this controller code:

    public IActionResult GetImage(int id)
    {
        byte[] imageBytes;
        try
        {
            imageBytes = _imageService.GetImageBytesById(id);
        }
        catch (CatalogImageMissingException ex)
        {
            _logger.LogWarning($"No image found for id: {id}");
            return NotFound();
        }
        return File(imageBytes, "image/png");
    }

I want to write a unit test that confirms a warning message is logged when the exception is thrown. I can easily mock the service to throw the exception. But I can't mock the _logger.LogWarning call because it's an extension method:

    [Fact]
    public void LogsWarningGivenImageMissingException()
    {
        SetupMissingImage();
        _mockLogger.Setup(l => l.LogWarning(It.IsAny<string>()))
            .Verifiable();

        _controller.GetImage(_testImageId);

        _mockLogger.Verify();
    }

This code fails at runtime:

Message: System.NotSupportedException : Expression references a method that does not belong to the mocked object: l => l.LogWarning(It.IsAny(), new[] { })

[PureKrome]

@pranavkm Question: why are these extension methods instead methods on the main interface ILogger?

[BrennanConroy]

@ardalis you can check ILogger.Log(It.Is<LogLevel>(LogLevel.Warning), ...)

@PureKrome Right now the ILogger interface is very small and neat, if you want to make a logger you only need to implement 3 methods, why would we want to force someone to implement the 24 extra extension methods everytime they inherit from ILogger?

[davidfowl]

@PureKrome Right now the ILogger interface is very small and neat, if you want to make a logger you only need to implement 3 methods, why would we want to force someone to implement the 24 extra extension methods everytime they inherit from ILogger?

I was going to echo this.

[PureKrome]

What is the recommended way to verify a particular Log method was hit?

logger.Verify(x => x.Log(It.Is<LogLevel>(LogLevel.Warning), ...) (as sorta suggested above by @BrennanConroy ?

[davidfowl]

That's the only way to do it. Don't get me wrong, it's a PITA but converting those extension methods to interface methods would be a disaster.

[PureKrome]

Cheers!

[ardalis]

I can see the reasoning to keep ILogger simple; thanks!

[ardalis]

For anybody else who stumbles onto this:
https://ardalis.com/testing-logging-in-aspnet-core

[PureKrome]

That's an interesting compromise @ardalis :) Nice!

[gokulm]

@ardalis thanks for the blog post!! extension methods aren't quite convincing, especially as it makes unit testing painful. At least Debug(string msg), Warn(string msg), Info(string msg), Error(string msg) and Error(Exception ex) could have been part of ILogger, as those are common methods that everybody expects to be part of any logging framework.