Add regression tests for 8 previously untested bug fixes

Tests verify the specific behavior that was fixed — each test would
fail if the corresponding fix were reverted.

- GH-4461: Socket IOException not propagated when child exits (2 tests)
- GH-2319: ErrorStackTrace settable before ErrorMessage (3 tests)
- GH-5132: WarnOnFileOverwrite parameter parsed and applied (2 tests)
- GH-4243: TestOutcome.Error is value 0, no Min/Max aliases (3 tests)
- GH-5184: Stderr forwarded as Informational, not Error (2 tests)
- GH-2479: ARM64 on Windows uses DLL hosting, not testhost.exe (1 test)
- GH-2483: HtmlLogger.Initialize creates results directory (2 tests)
- GH-3454: PortablePdbReader(MetadataReaderProvider) validates null (1 test)

Co-authored-by: Copilot <[email protected]>

Author: nohwnd

test/Microsoft.TestPlatform.CommunicationUtilities.UnitTests/RegressionBugFixTests.cs

@@ -0,0 +1,115 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.IO;
+using System.Net;
+using System.Net.Sockets;
+using System.Threading;
+using System.Threading.Tasks;
+
+using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
+using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.Interfaces;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+using Moq;
+
+namespace Microsoft.TestPlatform.CommunicationUtilities.UnitTests;
+
+/// <summary>
+/// Regression test for GH-4461:
+/// After a non-timeout IOException with a SocketException inner exception,
+/// the error output parameter must remain null (not the exception).
+/// Previously, the code set <c>error = ioException</c>, which propagated
+/// confusing socket errors to developers when testhost crashed.
+/// </summary>
+[TestClass]
+public class RegressionBugFixTests
+{
+    public TestContext TestContext { get; set; } = null!;
+
+    private static (TcpClient client, TcpClient serverSide, TcpListener listener) CreateConnectedTcpPair()
+    {
+        var listener = new TcpListener(IPAddress.Loopback, 0);
+        listener.Start();
+        var port = ((IPEndPoint)listener.LocalEndpoint).Port;
+
+        var client = new TcpClient();
+        client.Connect(IPAddress.Loopback, port);
+        var serverSide = listener.AcceptTcpClient();
+
+        return (client, serverSide, listener);
+    }
+
+    [TestMethod]
+    public async Task MessageLoopAsync_NonTimeoutSocketIOException_ErrorMustBeNull()
+    {
+        // GH-4461: When a non-timeout IOException wrapping a SocketException occurs,
+        // the error handler must receive null (not the exception).
+        // If the fix were reverted (error = ioException uncommented), capturedError would be non-null.
+        var (client, serverSide, listener) = CreateConnectedTcpPair();
+        try
+        {
+            var channel = new Mock<ICommunicationChannel>();
+            var socketException = new SocketException((int)SocketError.ConnectionReset);
+            var ioException = new IOException("Connection forcibly closed", socketException);
+
+            channel.Setup(c => c.NotifyDataAvailable(It.IsAny<CancellationToken>()))
+                .Throws(ioException);
+
+            Exception? capturedError = null;
+
+            // Write data so Poll returns true and NotifyDataAvailable is invoked.
+            var serverStream = serverSide.GetStream();
+            await serverStream.WriteAsync(new byte[] { 0x1 }, 0, 1, TestContext.CancellationToken);
+            await serverStream.FlushAsync(TestContext.CancellationToken);
+
+            using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.CancellationToken);
+            cts.CancelAfter(TimeSpan.FromSeconds(5));
+
+            await client.MessageLoopAsync(channel.Object, error => capturedError = error, cts.Token);
+
+            // The fix commented out `error = ioException`, so capturedError must be null.
+            Assert.IsNull(capturedError,
+                "GH-4461: Non-timeout IOException with SocketException must NOT be propagated to the error handler.");
+        }
+        finally
+        {
+            client.Dispose();
+            serverSide.Dispose();
+            listener.Stop();
+        }
+    }
+
+    [TestMethod]
+    public async Task MessageLoopAsync_NonIOException_ErrorMustBeSet()
+    {
+        // Contrast test: non-IOException exceptions must still be propagated.
+        var (client, serverSide, listener) = CreateConnectedTcpPair();
+        try
+        {
+            var channel = new Mock<ICommunicationChannel>();
+            var expectedException = new InvalidOperationException("Non-IO failure");
+            channel.Setup(c => c.NotifyDataAvailable(It.IsAny<CancellationToken>()))
+                .Throws(expectedException);
+
+            Exception? capturedError = null;
+
+            serverSide.GetStream().Write(new byte[] { 1 }, 0, 1);
+
+            using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.CancellationToken);
+            cts.CancelAfter(TimeSpan.FromSeconds(5));
+
+            await client.MessageLoopAsync(channel.Object, error => capturedError = error, cts.Token);
+
+            Assert.AreSame(expectedException, capturedError,
+                "Non-IOException must still be propagated to the error handler.");
+        }
+        finally
+        {
+            client.Dispose();
+            serverSide.Dispose();
+            listener.Stop();
+        }
+    }
+}

test/Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests/RegressionBugFixTests.cs

@@ -0,0 +1,76 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.IO;
+using System.Runtime.Serialization;
+
+using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
+using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+using Moq;
+
+using HtmlLoggerImpl = Microsoft.VisualStudio.TestPlatform.Extensions.HtmlLogger.HtmlLogger;
+using IHtmlTransformer = Microsoft.VisualStudio.TestPlatform.Extensions.HtmlLogger.IHtmlTransformer;
+
+namespace Microsoft.TestPlatform.Extensions.HtmlLogger.UnitTests;
+
+/// <summary>
+/// Regression test for GH-2483:
+/// HtmlLogger.Initialize must create the test results directory if it doesn't exist.
+/// Previously, Initialize would not create the directory, causing later file
+/// operations to fail with DirectoryNotFoundException.
+/// </summary>
+[TestClass]
+public class RegressionBugFixTests
+{
+    [TestMethod]
+    public void Initialize_NonExistentDirectory_MustCreateIt()
+    {
+        // GH-2483: The fix added Directory.CreateDirectory(testResultsDirPath) in Initialize.
+        // If the fix were reverted, the directory would not exist after Initialize.
+        var mockFileHelper = new Mock<IFileHelper>();
+        var mockHtmlTransformer = new Mock<IHtmlTransformer>();
+        var mockXmlSerializer = new Mock<XmlObjectSerializer>();
+        var htmlLogger = new HtmlLoggerImpl(mockFileHelper.Object, mockHtmlTransformer.Object, mockXmlSerializer.Object);
+
+        var events = new Mock<TestLoggerEvents>();
+        var nonExistentDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N"));
+
+        try
+        {
+            // Act
+            htmlLogger.Initialize(events.Object, nonExistentDir);
+
+            // Assert: directory must have been created
+            Assert.IsTrue(Directory.Exists(nonExistentDir),
+                "GH-2483: Initialize must call Directory.CreateDirectory for the test results directory.");
+            Assert.AreEqual(nonExistentDir, htmlLogger.TestResultsDirPath);
+        }
+        finally
+        {
+            if (Directory.Exists(nonExistentDir))
+            {
+                Directory.Delete(nonExistentDir);
+            }
+        }
+    }
+
+    [TestMethod]
+    public void Initialize_ExistingDirectory_MustNotThrow()
+    {
+        // Complementary test: Initialize with an existing directory must succeed.
+        var mockFileHelper = new Mock<IFileHelper>();
+        var mockHtmlTransformer = new Mock<IHtmlTransformer>();
+        var mockXmlSerializer = new Mock<XmlObjectSerializer>();
+        var htmlLogger = new HtmlLoggerImpl(mockFileHelper.Object, mockHtmlTransformer.Object, mockXmlSerializer.Object);
+
+        var events = new Mock<TestLoggerEvents>();
+        var existingDir = Path.GetTempPath();
+
+        htmlLogger.Initialize(events.Object, existingDir);
+
+        Assert.AreEqual(existingDir, htmlLogger.TestResultsDirPath);
+    }
+}

test/Microsoft.TestPlatform.Extensions.TrxLogger.UnitTests/RegressionBugFixTests.cs

@@ -0,0 +1,182 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Reflection;
+
+using Microsoft.TestPlatform.Extensions.TrxLogger.Utility;
+using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
+using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+using Moq;
+
+using TrxLoggerConstants = Microsoft.TestPlatform.Extensions.TrxLogger.Utility.Constants;
+using TrxLoggerObjectModel = Microsoft.TestPlatform.Extensions.TrxLogger.ObjectModel;
+
+using DefaultLoggerParameterNames = Microsoft.VisualStudio.TestPlatform.ObjectModel.DefaultLoggerParameterNames;
+
+namespace Microsoft.TestPlatform.Extensions.TrxLogger.UnitTests;
+
+/// <summary>
+/// Regression tests for:
+/// - GH-2319: Setting ErrorStackTrace before ErrorMessage must not crash.
+/// - GH-5132: WarnOnFileOverwrite=false must suppress overwrite warning.
+/// - GH-4243: TestOutcome.Error must have value 0, must serialize as "Error" (not "Min").
+/// </summary>
+[TestClass]
+public class RegressionBugFixTests
+{
+    private static readonly string DefaultTestRunDirectory = Path.GetTempPath();
+
+    #region GH-2319: ErrorStackTrace without ErrorMessage doesn't crash
+
+    [TestMethod]
+    public void ErrorStackTrace_SetBeforeErrorMessage_MustNotThrow()
+    {
+        // GH-2319: Previously, setting ErrorStackTrace first would crash with Debug.Assert
+        // because _errorInfo was null. The fix uses ??= to lazily initialize.
+        var testResult = CreateTestResult();
+
+        // Act: set ErrorStackTrace FIRST, before any ErrorMessage
+        testResult.ErrorStackTrace = "at SomeTest.Method() in file.cs:line 42";
+
+        // Assert: should not throw and ErrorMessage should return empty string (not null or crash)
+        Assert.AreEqual("at SomeTest.Method() in file.cs:line 42", testResult.ErrorStackTrace);
+        Assert.AreEqual(string.Empty, testResult.ErrorMessage,
+            "GH-2319: ErrorMessage must return empty string when only ErrorStackTrace is set.");
+    }
+
+    [TestMethod]
+    public void ErrorMessage_ThenErrorStackTrace_BothAccessible()
+    {
+        // Setting ErrorMessage first, then ErrorStackTrace: both must be readable.
+        var testResult = CreateTestResult();
+
+        testResult.ErrorMessage = "Assert.Fail hit";
+        testResult.ErrorStackTrace = "at MyTest.Run()";
+
+        Assert.AreEqual("Assert.Fail hit", testResult.ErrorMessage);
+        Assert.AreEqual("at MyTest.Run()", testResult.ErrorStackTrace);
+    }
+
+    [TestMethod]
+    public void ErrorMessage_NeverSet_ReturnsEmptyString()
+    {
+        // Before the fix, accessing getters on a fresh TestResult without _errorInfo
+        // would behave unpredictably. The fix returns string.Empty via null-coalescing.
+        var testResult = CreateTestResult();
+
+        Assert.AreEqual(string.Empty, testResult.ErrorMessage);
+        Assert.AreEqual(string.Empty, testResult.ErrorStackTrace);
+    }
+
+    #endregion
+
+    #region GH-5132: WarnOnFileOverwrite parameter
+
+    [TestMethod]
+    public void Initialize_WarnOnFileOverwriteFalse_FieldMustBeFalse()
+    {
+        // GH-5132: When WarnOnFileOverwrite=false, the _warnOnFileOverwrite field must be false.
+        // If the fix were reverted (field removed), this would fail.
+        var logger = new TestableTrxLogger();
+        var events = new Mock<TestLoggerEvents>();
+        var parameters = CreateDefaultParameters();
+        parameters[TrxLoggerConstants.WarnOnFileOverwrite] = "false";
+
+        logger.Initialize(events.Object, parameters);
+
+        var warnField = typeof(VisualStudio.TestPlatform.Extensions.TrxLogger.TrxLogger)
+            .GetField("_warnOnFileOverwrite", BindingFlags.NonPublic | BindingFlags.Instance);
+        Assert.IsNotNull(warnField, "_warnOnFileOverwrite field must exist.");
+        Assert.IsFalse((bool)warnField.GetValue(logger)!,
+            "GH-5132: _warnOnFileOverwrite must be false when parameter is 'false'.");
+    }
+
+    [TestMethod]
+    public void Initialize_WarnOnFileOverwriteNotSet_DefaultsToTrue()
+    {
+        // GH-5132: When WarnOnFileOverwrite parameter is not provided, default must be true
+        // (preserving existing behavior for users who did not opt out).
+        var logger = new TestableTrxLogger();
+        var events = new Mock<TestLoggerEvents>();
+        var parameters = CreateDefaultParameters();
+
+        logger.Initialize(events.Object, parameters);
+
+        var warnField = typeof(VisualStudio.TestPlatform.Extensions.TrxLogger.TrxLogger)
+            .GetField("_warnOnFileOverwrite", BindingFlags.NonPublic | BindingFlags.Instance);
+        Assert.IsNotNull(warnField, "_warnOnFileOverwrite field must exist.");
+        Assert.IsTrue((bool)warnField.GetValue(logger)!,
+            "GH-5132: _warnOnFileOverwrite must default to true when parameter is not provided.");
+    }
+
+    #endregion
+
+    #region GH-4243: TestOutcome has no Min/Max aliases
+
+    [TestMethod]
+    public void TestOutcome_Error_MustHaveIntValue0()
+    {
+        // GH-4243: Error must be the first enum member (value 0).
+        // Previously Min=Error alias made Error.ToString() return "Min".
+        // Box through object to avoid compile-time constant folding (MSTEST0032).
+        object errorEnum = TrxLoggerObjectModel.TestOutcome.Error;
+        Assert.AreEqual(0, (int)(TrxLoggerObjectModel.TestOutcome)errorEnum,
+            "GH-4243: TestOutcome.Error must have integer value 0.");
+    }
+
+    [TestMethod]
+    public void TestOutcome_Error_MustSerializeAsError_NotMin()
+    {
+        // GH-4243: The fix removed Min=Error alias. Error.ToString() must return "Error".
+        // If the fix were reverted and Min=Error added back, ToString() would return "Min".
+        Assert.AreEqual("Error", TrxLoggerObjectModel.TestOutcome.Error.ToString(),
+            "GH-4243: TestOutcome.Error.ToString() must be 'Error', not 'Min'.");
+    }
+
+    [TestMethod]
+    public void TestOutcome_NoMinOrMaxMember()
+    {
+        // GH-4243: The enum must NOT have a member named "Min" or "Max".
+        // Verify via reflection.
+        var names = Enum.GetNames(typeof(TrxLoggerObjectModel.TestOutcome));
+        CollectionAssert.DoesNotContain(names, "Min",
+            "GH-4243: TestOutcome enum must not have a 'Min' member.");
+        CollectionAssert.DoesNotContain(names, "Max",
+            "GH-4243: TestOutcome enum must not have a 'Max' member.");
+    }
+
+    #endregion
+
+    #region Helpers
+
+    private static TrxLoggerObjectModel.TestResult CreateTestResult()
+    {
+        return new TrxLoggerObjectModel.TestResult(
+            runId: Guid.NewGuid(),
+            testId: Guid.NewGuid(),
+            executionId: Guid.NewGuid(),
+            parentExecutionId: Guid.Empty,
+            resultName: "TestResult1",
+            computerName: Environment.MachineName,
+            outcome: TrxLoggerObjectModel.TestOutcome.Failed,
+            testType: new TrxLoggerObjectModel.TestType(Guid.NewGuid()),
+            testCategoryId: TrxLoggerObjectModel.TestListCategoryId.Uncategorized,
+            trxFileHelper: new TrxFileHelper());
+    }
+
+    private static Dictionary<string, string?> CreateDefaultParameters()
+    {
+        return new Dictionary<string, string?>
+        {
+            [DefaultLoggerParameterNames.TestRunDirectory] = DefaultTestRunDirectory,
+            [TrxLoggerConstants.LogFileNameKey] = "test.trx"
+        };
+    }
+
+    #endregion
+}