Ensure we properly dispose our loggers during MSBuildWorkspace tests (#82942)

xunit enforces that if you try to write to ITestOutputHelper after the
end of a test run that the test run is failed. Our ILoggerProvider
implementation already handles this -- once it's disposed, it stops
sending messages along as a way to not have a failing test (which
accidentally runs stuff asynchronously) from killing an entire test run.
We however had cases in the MSBuildWorkspace tests were we just forgot
to dispose it.

Fixes #82939
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/dotnet/roslyn/pull/82942)

Author: jasonmalinowski

src/Workspaces/CoreTestUtilities/Logging/TestOutputLoggerProvider.cs

@@ -51,6 +51,18 @@ public void Dispose()
         }
     }
 
+    /// <summary>
+    /// Validates that this provider has not already been disposed, and then disposes it. This is useful in
+    /// test teardown to catch bugs where the provider was disposed too early and we might lose messages.
+    /// </summary>
+    public void ValidateNotAlreadyDisposedAndDispose()
+    {
+        if (_testOutputHelper is null)
+            throw new ObjectDisposedException(nameof(TestOutputLoggerProvider));
+
+        Dispose();
+    }
+
     public void Dispose()
     {
         _testOutputHelper = null;

src/Workspaces/MSBuild/Core/MSBuild/MSBuildWorkspace.cs

@@ -96,6 +96,13 @@ private DiagnosticReporter Reporter
     internal void AddLoggerProvider(Microsoft.Extensions.Logging.ILoggerProvider loggerProvider)
         => _loader.LoggerFactory.AddProvider(loggerProvider);
 
+    protected override void Dispose(bool finalize)
+    {
+        // Dispose the LoggerFactory to ensure any logger providers added via AddLoggerProvider are disposed.
+        _loader.LoggerFactory.Dispose();
+        base.Dispose(finalize);
+    }
+
     /// <summary>
     /// The MSBuild properties used when interpreting project files.
     /// These are the same properties that are passed to msbuild via the /property:&lt;n&gt;=&lt;v&gt; command line argument.

src/Workspaces/MSBuild/Test/MSBuildWorkspaceTestBase.cs

@@ -24,13 +24,31 @@ namespace Microsoft.CodeAnalysis.MSBuild.UnitTests;
 
 public abstract class MSBuildWorkspaceTestBase : WorkspaceTestBase
 {
-    protected readonly ITestOutputHelper TestOutput;
+    private readonly ITestOutputHelper _testOutputHelper;
+    private readonly TestOutputLoggerProvider _testOutputLoggerProvider;
     protected readonly ILoggerFactory LoggerFactory;
 
     protected MSBuildWorkspaceTestBase(ITestOutputHelper testOutput)
     {
-        TestOutput = testOutput;
-        LoggerFactory = new LoggerFactory([new TestOutputLoggerProvider(testOutput)]);
+        _testOutputHelper = testOutput;
+        _testOutputLoggerProvider = new TestOutputLoggerProvider(testOutput);
+        LoggerFactory = new LoggerFactory([_testOutputLoggerProvider]);
+    }
+
+    public override void Dispose()
+    {
+        // Dispose our LoggingFactory and providers. xunit validates that we don't write anything to ITestOutputHelper after a test is done,
+        // so we want to ensure our providers are all disposed so a broken test doesn't cause the entire test run to fail -- our implementation of
+        // TestOutputLoggerProvider stops forwarding messages once it's disposed.
+        //
+        // LoggerFactory's handling of lifetime is subtle -- providers passed to the LoggerFactorys' constructor are not owned and we have to dispose them;
+        // but providers added via AddLoggerProvider are owned and will be disposed by the LoggerFactory. Thus we need to dispose both here.
+        LoggerFactory.Dispose();
+
+        // We'll call ValidateNotAlreadyDisposedAndDispose() as a way to ensure this wasn't disposed prematurely, which would cause us to lose log output.
+        _testOutputLoggerProvider.ValidateNotAlreadyDisposedAndDispose();
+
+        base.Dispose();
     }
 
     protected const string MSBuildNamespace = "http://schemas.microsoft.com/developer/msbuild/2003";
@@ -160,7 +178,7 @@ protected MSBuildWorkspace CreateMSBuildWorkspace(
     {
         additionalProperties ??= [];
         var workspace = MSBuildWorkspace.Create(CreateProperties(additionalProperties));
-        workspace.AddLoggerProvider(new TestOutputLoggerProvider(TestOutput));
+        workspace.AddLoggerProvider(new TestOutputLoggerProvider(_testOutputHelper));
 
         if (throwOnWorkspaceFailed)
         {

src/Workspaces/MSBuild/Test/NewlyCreatedProjectsFromDotNetNew.cs

@@ -147,7 +147,8 @@ private async Task AssertTemplateProjectLoadsCleanlyAsync(string templateName, s
     {
         if (ignoredDiagnostics?.Length > 0)
         {
-            TestOutput.WriteLine($"""
+            var logger = LoggerFactory.CreateLogger(nameof(AssertTemplateProjectLoadsCleanlyAsync));
+            logger.LogInformation($"""
                 Ignoring compiler diagnostics: "{string.Join("\", \"", ignoredDiagnostics)}"
                 """);
         }