Add Allure.Reqnroll test output handler support

[coman3]

Context

Usage of the built in reqnroll test output helper was ignored, and no event was in place to handle these outputs. As text based test output can be extremely useful to help diagnose issues, this was a hard requirement for us to migrate from SpecFlow and LivingDoc to Reqnroll and Allure Reports.

Checklist

• [Sign Allure CLA][cla]
• Provide unit tests TBD

Unit tests are still to be updated, i just wanted to get opinions on how better this could potentially be implemented.

Notes

I really didn't want to modify the Allure.Net.Commons project in order to enable support for this, however i did want to ensure that test outputs can be captured in as many cases as possible, without each write to these outputs being placed into a single file - hence the OutputCache static property that I really didn't want to need.

Any suggestions / improvements here are of course welcome. This PR is a major step to towards us being able to implement Allure instead of LivingDoc for our team, so hopefully we can figure out a way to get this in, otherwise we might have to fork this 😊

Usage of the ExecutableItem as a key for the dictionary ensures support for parallel test executions within a container, however there is limitations on a single container I can't seem to identify exactly the purpose of the container, or when multiple containers might exist, any advice here would be great, and I can then adapt this to work for many containers if required.

Preview

A preview of what this looks like in the test overview is like so:

Usage is compatible with existing ReqnRoll / SpecFlow output helper:

    readonly ITestOutputHelper outputHelper;
    public SomeClass(ITestOutputHelper outputHelper)
    {
        this.outputHelper = outputHelper;
    }
    [StepDefinition("Step with test output")] //Note that AfterFeature, AfterScenario, AfterStep (and before) all work as well
    public void StepWithTestOutput() => outputHelper.WriteLine("Test output message");

[CLAassistant]

All committers have signed the CLA.

[coman3]

I typically try to avoid static dictionaries, especially in parallel execution contexts, however i couldn't see a path forward not using something like this, without modifying the Allure.Net.Common library.

[delatrie]

That's a valid concern. Please take a look at my review comment for the alternative.

[delatrie]

Hi, @coman3 ! Thank you for your contribution. This is definitely a feature we want to have. I agree we should scope it at the Allure.Reqnroll level for now, because a general abstraction needs more design effort than a Reqnroll-specific implementation.

I will take a closer look soon and provide you with my feedback!

[coman3]

hey @delatrie, would love a quick update on this if you have a chance 😊

[delatrie]

Hello, @coman3!

Sorry for having kept you waiting.

I took a look, and I like your approach. It's simple and works well both with steps and hooks.

Let's fix some concurrency issues, though:

• Access to StringBuilder should be wrapped in lock.
• Use ConcurrentDictionary and its GetOrAdd and TryRemove methods instead of plain Dictionary.
• Use ExecutionItem.uuid as keys instead of ExecutionItem instances themselves.

May I also ask you to factor out the code related to the output handling to a separate class please? Something like TestOutputCache with the following methods:

• AddOutput(string text)
• AttachOutput()
• Clear()

Here is how to use the concurrent dictionary inside those methods:

public void AddOutput(string text)
{
    var sb = this.outputCache.GetOrAdd(GetKey(), () => new StringBuilder());
    lock (sb)
    {
        sb.AppendLine(text);
    }
}

public void AttachOutput()
{
    var output = "";
    if (this.outputCache.TryRemove(GetKey(), out var sb))
    {
        lock (sb)
        {
            output = sb.ToString();
        }
    }

    /* the rest of the code */
}

static string GetKey()
{
    string key = default;
    AllureLifecycle.Instance.UpdateExecutableItem(tr => { key = tr.uuid; });
    return key;
}

A static instance of that class can then be put in the state facade.

[delatrie]

Since this is the only place where AddOutput is called, we may reduce the number of concatenations by using StringBuilder's AppendLine instead of appending the new line character here and calling Append.

[delatrie]

That's a valid concern. Please take a look at my review comment for the alternative.