[FEATURE]: Improve xUnit Logging capabilities

[thePantz]

Description

I've run into a problem using Boa with xUnit. Because of the way that xUnit manages shared context between test classes....
xUnit injects ITestOutputHelper into the test class constructor. Which I provide to Boas XUnitLogger like so

public MyTestClass(ITestOutputHelper output)
{
    this.actor = new Actor("Pantz", new XunitLogger(output));
}

This works fine until I want to create a test fixture to share context across multiple test classes.

public class DatabaseFixture : IDisposable
{
    public DatabaseFixture()
    {
        var connection = new SqlConnection("MyConnectionString");

        // ... initialize data in the test database ...

        MyActor = new Actor("Pantz", new XunitLogger(**???**));
        MyActor.Can(ConnectToDatabase.Using(connection);
    }

    public void Dispose()
    {
        // ... clean up test data from the database ...
    }

    public Actor MyActor { get; private set; }
}

public class MyDatabaseTests : IClassFixture<DatabaseFixture>
{
    DatabaseFixture fixture;

    public MyDatabaseTests(DatabaseFixture fixture)
    {
        this.fixture = fixture;
    }

    // ... write tests, using fixture.MyActor to get access to the SQL Server ...
}

It's impossible to instantiate an actor with the logger in the fixture since xUnit does not provide the output helper here. I believe the reason is because of the way xUnit handles parallel execution, the output helper keeps track of the currently executing test

I think need some way to add the logger after the actor has been initialized... from what I can tell this is not currently possible since actor.Logger doesn't have a public setter

Can we make the property public?

Alternatives

1. Make Logger setter internal.
2. Create some kind of ability that instantiates an ILogger
3. Modify the Logger getter to check if the actor has the ability and get the instance of ILogger

Anything else?

No response

Commitments

• I agree to follow Boa Constrictor's Code of Conduct.
• I want to work on this issue myself. (This is voluntary, not required.)

[AutomationPanda]

I'm a bit hesitant to give Actor's Logger property a public setter.

What if we added a public setter to XunitLogger's TestOutputHelper property? Then, the example code above could be written like this:

public class DatabaseFixture : IDisposable
{
    public DatabaseFixture()
    {
        MyXunitLogger = new XunitLogger(null);
        MyActor = new Actor("Pantz", MyXunitLogger);
        MyActor.Can(ConnectToDatabase.Using(connection);
    }

    public Actor MyActor { get; private set; }
    public Actor MyXunitLogger { get; private set; }
}

Then, you could change the ITestOutputHelper for the XunitLogger in the test.

(I presume the fixture would run once per test and would be thread safe.)

[thePantz]

Understandable, I played around with making the logger public but it will probably cause more issues / encourage bad design decisions...

Your proposed solution won't work either unfortunately...
xUnits order of operations is:

• Fixture constructor
• Test class constructor <-- injects ITestOutputHelper
• Test class dispose
• fixture dispose

So when MyActor.Can(ConnectToDatabase.Using(connection); is called, nothing will be logged because the outputHelper will not have been set yet.

I'm starting to second guess myself here... I think it may be a bad idea to have a shared actor anyhow. In the above example I could use the fixture to create a shared database connection. Then just setup the actor in the test class

public class DatabaseFixture : IDisposable
{
    public DatabaseFixture()
    {
        Connection = new SqlConnection("MyConnectionString");

        // ... initialize data in the test database ...
    }

    public void Dispose()
    {
        // ... clean up test data from the database ...
    }

    public SqlConnection Connection{ get; private set; }
}

public class MyDatabaseTests : IClassFixture<DatabaseFixture>
{
    private Actor DatabaseActor;
    public MyDatabaseTests(DatabaseFixture fixture, ITestOutputHelper output)
    {
        var connection = fixture.Connection
        this.DatabaseActor = new Actor("Pantz", new XunitLogger(output));
        this.DatabaseActor.Can(ConnectToDatabase.Using(connection);
    }

    // ... write tests, using DatabaseActor to get access to the SQL Server ...
}

I could cleanup this implementation a bit by making a custom actor object.

---

What I might do instead is add a new logger to support IMessageSink.
It's a different logger that xUnit inject into the fixture and a few other extensibility classes it has.

    /// <summary>
    /// Prints messages to xUnit's IMessageSink.
    /// </summary>
    public class MessageSinkLogger : AbstractLogger
    {
        #region Constructors

        /// <summary>
        /// Constructor.
        /// </summary>
        /// <param name="messageSink">the logger object used by xUnit's extensibility classes.</param>
        /// <param name="lowestSeverity">The lowest severity message to log.</param>
        public MessageSinkLogger(IMessageSink messageSink, LogSeverity lowestSeverity = LogSeverity.Info)
            :base(lowestSeverity)
        {
            MessageSink = messageSink;
        }

        #endregion

        #region Properties

        /// <summary>
        /// A logger object used by xUnit's extensibility classes.
        /// </summary>
        public IMessageSink MessageSink { get; set; }

        #endregion

        #region Log Method Implementations

        /// <summary>
        /// Closes the logging stream
        /// (No-op for IMessageSink)
        /// </summary>
        public override void Close()
        {
        }

        /// <summary>
        /// Logs a basic message to the console after checking the lowest severity.
        /// </summary>
        /// <param name="message">The message text.</param>
        /// <param name="severity">The severity level (defaults to info).</param>
        protected override void LogRaw(string message, LogSeverity severity = LogSeverity.Info)
        {
            if (severity >= LowestSeverity)
            {
                var diagnosticMessage = new DiagnosticMessage(message);
                MessageSink.OnMessage(diagnosticMessage);
            }
        }

        #endregion
    }

So if I wanted to run some screenplay tasks in the fixture and have them logged, I could do something like:

    public TestFixture(IMessageSink messageSink)
    {
        Pantz = new Actor("Pantz", new MessageSinkLogger(messageSink));
        Pantz.Logger.Log("Fixture Constructor");
    }

    public void Dispose()
    {
        Pantz.Logger.Log("Fixture Dispose");
    }

    private Actor Pantz { get; }

note that the actor is private in this case. It should not be used outside of the fixture, instead just make a new one...

The unfortunate thing about IMessageSink is that it doesn't write to the test output, but the test-runner diagnostic output. So these logs will be harder to find but at least they're somewhere...

[thePantz]

Also found this thread: xunit/xunit#565 (comment)

People have been complaining about MessageSink for this exact use case...

[AutomationPanda]

I think I like your suggestion of making the database connection in the fixture and creating the Actor in the test. Creating one Actor per test helps preserve test case independence.