serilog-contrib / SerilogSinksInMemory

In-memory sink for Serilog to use for testing
MIT License
53 stars 7 forks source link

Updating to Fluent Assertions 6.0 #22

Closed jnick422 closed 2 years ago

jnick422 commented 2 years ago

Recently updated a repository to Fluent Assertions 6.0 and was running into some issues with the InMemoryAssertions:

 Message: 
    System.MissingMethodException : Method not found: 'Void FluentAssertions.Primitives.ReferenceTypeAssertions`2..ctor()'.

  Stack Trace: 
    InMemorySinkAssertions.ctor(InMemorySink instance)
    InMemorySinkAssertionExtensions.Should(InMemorySink instance)
    CreateFromContractMessageHandlerTests.ShouldLogIfInvalidAccountName(Mock`1 s3Client) line 344
    --- End of stack trace from previous location ---

When making a call like so:

InMemorySink.Instance.Should()
    .HaveErrorMessageWithException<ArgumentException>(
        "An invalid argument was encountered while processing message with ID {MessageId}",
        "Account name cannot be longer than 100 characters");

This is what the error message extension method looks like:

      public static void HaveErrorMessageWithException<T>(this InMemorySinkAssertions assertion, string messageTemplate, string innerMessage = null)
      {
          if (innerMessage == null)
          {
              assertion.HaveMessage(messageTemplate)
                  .WithLevel(LogEventLevel.Error)
                  .Appearing().Once()
                  .Match(o => o.Exception.GetType() == typeof(T));
          }

          assertion.HaveMessage(messageTemplate)
              .WithLevel(LogEventLevel.Error)
              .Appearing().Once()
              .Match(o => o.Exception.GetType() == typeof(T) &&
                      o.Exception.Message.Contains(innerMessage)
              );
      }

This was working with Fluent Assertions 5.10.3. Let me know if you need any more information

sandermvanvliet commented 2 years ago

This is not uncommon when FluentAssertiosn has an upgrade (and especially with major version updates). I can take a look at this and see if we can make it compatible between versions of FluentAssertions

siewers commented 2 years ago

Any update on this or would you be accepting a pull request? I've looked at the implementation, and there doesn't seem to be a way to make it compatible with the old version. The problem seems to come from the fact that all assertions inherit from a base class, which now requires the subject to be passed. Of course, it could be done using compiler directives, but is that preferred?

sandermvanvliet commented 2 years ago

@siewers @jnick422 sorry it took a while to get some time to do this. I've managed to make this work without having to break compatibility with FluentAssertions 5.

However I do need to sort out some things with the build after moving this repo into serilog-contrib so it might take some time before this version hits NuGet.

sandermvanvliet commented 2 years ago

For reference, I've created this test to reproduce the issue as reported by @jnick422 and this now passes for both FluentAssertions 5 and 6:

public class Repro
{
    [Fact]
    public void FromIssue22()
    {
        var logger = new LoggerConfiguration()
            .WriteTo.InMemory()
            .CreateLogger();

        logger.Error(
            new ArgumentException("Account name cannot be longer than 100 characters"),
            "An invalid argument was encountered while processing message with ID {MessageId}", "some-message-id");

        InMemorySink.Instance.Should()
            .HaveErrorMessageWithException<ArgumentException>(
                "An invalid argument was encountered while processing message with ID {MessageId}",
                "Account name cannot be longer than 100 characters");
    }
}

public static class Extension
{
    public static void HaveErrorMessageWithException<T>(this InMemorySinkAssertions assertion, string messageTemplate, string innerMessage = null)
    {
        if (innerMessage == null)
        {
            assertion.HaveMessage(messageTemplate)
                .WithLevel(LogEventLevel.Error)
                .Appearing().Once()
                .Match(o => o.Exception.GetType() == typeof(T));
        }

        assertion.HaveMessage(messageTemplate)
            .WithLevel(LogEventLevel.Error)
            .Appearing().Once()
            .Match(o => o.Exception.GetType() == typeof(T) &&
                    o.Exception.Message.Contains(innerMessage)
            );
    }
}
sandermvanvliet commented 2 years ago

@jnick422 @siewers I've fixed the build issues and version 0.9.0 of the assertions package has just been published to NuGet so you should see it appearing in the NuGet feed shortly.

Thanks for bringing this issue to my attention and happy testing :+1: