Refactor Formatters to make Shutdown explicit

[clrudolphi]

🤔 What's changed?

The interface definitions of the Broker and FormatterBase have been modified to add explicit methods for closing.

Sending the TestRunFinished message is no longer an implicit signal to shutdown the formatters.

The Publisher will call the Broker's CompleteAsync() method which will forward that call to each active formatter by invoking their CloseAsync() methods.

⚡️ What's your motivation?

Makes the design more explicit.

🏷️ What kind of change is this?

• 💥 Breaking change (incompatible changes to the API) (adding methods to existing interfaces)

♻️ Anything particular you want feedback on?

📋 Checklist:

• I've changed the behaviour of the code
  • I have added/updated tests to cover my changes.
• My change requires a change to the documentation.
  • I have updated the documentation accordingly.
• Users should know about my change
  • I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

---

This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.

[Code-Grump]

Was IAsyncDisposable.DisposeAsync considered?

[clrudolphi]

Not for this PR, no. The intent of this PR is to make Publisher/Broker/Formatter shutdown explicit within the cooperation protocol among them. This call sequence when the test framework has caused the fixture shutdown to happen. That might, or might not, be within a disposal, depending upon the test framework.

But your comment spurs me to consider DisposeAsync for the other PR #1032

[clrudolphi]

Marked this PR as draft; I would like to wait for feedback from @MrSoerenAndersen on discussion item #1030 which might lead to further changes/additions to this. In particular, @MrSoerenAndersen, would it be helpful to add

public abstract Task OnCloseRequestedAsync();
public abstract Task OnCloseCompletedAsync();

to FormatterBase so that implementers of formatters can add before/after shutdown logic?

It might be just as simple to mark CloseAsync() as virtual and let derived formatters override it.

[MrSoerenAndersen]

To mark CloseAsync() as virtual would be sufficient for Expressium LivingDoc as long as the final NDJSON file is closed...

• As I understand this will omit the ExpressiumMessageFormatter class on my side...?
• And the final implementation will look like this...?

    public class ExpressiumFormatter : BaseFormatter
    {
        public string OutputFilePath { get; private set; }
        public string OutputFileTitle { get; private set; }

        public ExpressiumFormatter(IFormattersConfigurationProvider configurationProvider, IFormatterLog logger, IFileSystem fileSystem) : base(configurationProvider, logger, fileSystem, "expressium")
        {
        }

        protected override void FinalizeInitialization(string outputPath, IDictionary<string, object> formatterConfiguration, Action<bool> onInitialized)
        {
            base.FinalizeInitialization(outputPath, formatterConfiguration, onInitialized);
            OutputFilePath = outputPath;

            if (formatterConfiguration.ContainsKey("outputFileTitle"))
                OutputFileTitle = formatterConfiguration["outputFileTitle"].ToString();
        }

        public override async Task CloseAsync()
        {
            var outputHtmlFilePath = OutputFilePath.Replace(Path.GetExtension(OutputFilePath), ".html");

            var livingDocConverter = new LivingDocConverter();
            livingDocConverter.Generate(OutputFilePath, outputHtmlFilePath, OutputFileTitle);
        }
    }

[clrudolphi]

• And the final implementation will look like this...?

Almost. Call base first...

   await base.CloseAsync();

   # ... the rest of the override