Fixed testhost crash for net7

src/Microsoft.TestPlatform.Client/AttachmentsProcessing/InProcessTestRunAttachmentsProcessingEventsHandler.cs

@@ -0,0 +1,64 @@
+// 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 Microsoft.VisualStudio.TestPlatform.ObjectModel;
+using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
+using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
+
+namespace Microsoft.VisualStudio.TestPlatform.Client;
+
+internal class InProcessTestRunAttachmentsProcessingEventsHandler : ITestRunAttachmentsProcessingEventsHandler
+{
+    private readonly ITestRunAttachmentsProcessingEventsHandler _oldEventsHandler;
+
+    public InProcessTestRunAttachmentsProcessingEventsHandler(
+        ITestRunAttachmentsProcessingEventsHandler oldEventsHandler)
+    {
+        _oldEventsHandler = oldEventsHandler;
+    }
+
+    public void HandleLogMessage(TestMessageLevel level, string? message)
+    {
+        _oldEventsHandler.HandleLogMessage(level, message);
+    }
+
+    public void HandleProcessedAttachmentsChunk(IEnumerable<AttachmentSet> attachments)
+    {
+        // Not implemented by design, keep in sync with the same named method from
+        // TestRunAttachmentsProcessingEventsHandler.cs.
+        throw new NotImplementedException();
+    }
+
+    public void HandleRawMessage(string rawMessage)
+    {
+        // No-op by design.
+        //
+        // For out-of-process vstest.console, raw messages are passed to the translation layer but
+        // they are never read and don't get passed to the actual events handler in TW. If they
+        // were (as it happens for in-process vstest.console since there is no more translation
+        // layer) a NotImplemented exception would be raised as per the time this of writing this
+        // note.
+        //
+        // Consider changing this logic in the future if TW changes the handling logic for raw
+        // messages.
+    }
+
+    public void HandleTestRunAttachmentsProcessingComplete(
+        TestRunAttachmentsProcessingCompleteEventArgs attachmentsProcessingCompleteEventArgs,
+        IEnumerable<AttachmentSet>? lastChunk)
+    {
+        _oldEventsHandler.HandleTestRunAttachmentsProcessingComplete(
+            attachmentsProcessingCompleteEventArgs,
+            lastChunk);
+    }
+
+    public void HandleTestRunAttachmentsProcessingProgress(
+        TestRunAttachmentsProcessingProgressEventArgs attachmentsProcessingProgressEventArgs)
+    {
+        _oldEventsHandler.HandleTestRunAttachmentsProcessingProgress(
+            attachmentsProcessingProgressEventArgs);
+    }
+}

src/Microsoft.TestPlatform.Client/TestSession/InProcessTestSessionEventsHandler.cs

@@ -28,7 +28,16 @@ public void HandleLogMessage(TestMessageLevel level, string? message)
 
     public void HandleRawMessage(string rawMessage)
     {
-        _testSessionEventsHandler.HandleRawMessage(rawMessage);
+        // No-op by design.
+        //
+        // For out-of-process vstest.console, raw messages are passed to the translation layer but
+        // they are never read and don't get passed to the actual events handler in TW. If they
+        // were (as it happens for in-process vstest.console since there is no more translation
+        // layer) a NotImplemented exception would be raised as per the time this of writing this
+        // note.
+        //
+        // Consider changing this logic in the future if TW changes the handling logic for raw
+        // messages.
     }
 
     public void HandleStartTestSessionComplete(StartTestSessionCompleteEventArgs? eventArgs)

src/Microsoft.TestPlatform.PlatformAbstractions/PublicAPI/PublicAPI.Shipped.txt

@@ -120,4 +120,6 @@ static Microsoft.VisualStudio.TestPlatform.ObjectModel.PlatformEqtTrace.ErrorOnI
 static Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.PlatformAssemblyExtensions.GetAssemblyLocation(this System.Reflection.Assembly! assembly) -> string!
 Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces.IProcessHelper.GetProcessArchitecture(int processId) -> Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.PlatformArchitecture
 Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.ProcessHelper.GetProcessArchitecture(int processId) -> Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.PlatformArchitecture
+static Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.ProcessHelper.ExternalEnvironmentVariables.get -> System.Collections.Generic.IDictionary<string!, string?>?
+static Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.ProcessHelper.ExternalEnvironmentVariables.set -> void
 

src/Microsoft.TestPlatform.PlatformAbstractions/common/System/ProcessHelper.cs

@@ -40,6 +40,13 @@ internal ProcessHelper(IEnvironment environment)
         _environment = environment;
     }
 
+    /// <summary>
+    /// Gets or sets the set of environment variables to be used when spawning a new process.
+    /// Should this set of environment variables be null, the environment variables inherited from
+    /// the parent process will be used.
+    /// </summary>
+    public static IDictionary<string, string?>? ExternalEnvironmentVariables { get; set; }
+
     /// <inheritdoc/>
     public object LaunchProcess(string processPath, string? arguments, string? workingDirectory, IDictionary<string, string?>? envVariables, Action<object?, string?>? errorCallback, Action<object?>? exitCallBack, Action<object?, string?>? outputCallBack)
     {
@@ -77,6 +84,23 @@ void InitializeAndStart()
 
             process.EnableRaisingEvents = true;
 
+            // Resetting the baseline environment variables inherited from the parent process and
+            // replacing them with the desired set of environment variables.
+            if (ExternalEnvironmentVariables is not null)
+            {
+                process.StartInfo.EnvironmentVariables.Clear();
+                foreach (var kvp in ExternalEnvironmentVariables)
+                {
+                    if (kvp.Value is null)
+                    {
+                        continue;
+                    }
+
+                    process.StartInfo.AddEnvironmentVariable(kvp.Key, kvp.Value);
+                }
+            }
+
+            // Set additional environment variables.
             if (envVariables != null)
             {
                 foreach (var kvp in envVariables)

src/vstest.console/HandlerToEventsRegistrarAdapter.cs

@@ -23,7 +23,19 @@ public DiscoveryHandlerToEventsRegistrarAdapter(ITestDiscoveryEventsHandler2 han
         _handleDiscoveredTests += (_, e) => _handler.HandleDiscoveredTests(e.DiscoveredTestCases);
         _handleLogMessage += (_, e) => _handler.HandleLogMessage(e.Level, e.Message);
         _handleDiscoveryComplete += (_, e) => _handler.HandleDiscoveryComplete(e, null);
-        _handleRawMessage += (_, e) => _handler.HandleRawMessage(e);
+        _handleRawMessage += (_, e) =>
+        {
+            // No-op by design.
+            //
+            // For out-of-process vstest.console, raw messages are passed to the translation layer but
+            // they are never read and don't get passed to the actual events handler in TW. If they
+            // were (as it happens for in-process vstest.console since there is no more translation
+            // layer) a NotImplemented exception would be raised as per the time this of writing this
+            // note.
+            //
+            // Consider changing this logic in the future if TW changes the handling logic for raw
+            // messages.
+        };
     }
 
     public void LogWarning(string message)
@@ -60,7 +72,19 @@ public RunHandlerToEventsRegistrarAdapter(ITestRunEventsHandler handler)
     {
         _handler = handler;
         _handleLogMessage = (_, e) => _handler.HandleLogMessage(e.Level, e.Message);
-        _handleRawMessage = (_, e) => _handler.HandleRawMessage(e);
+        _handleRawMessage = (_, e) =>
+        {
+            // No-op by design.
+            //
+            // For out-of-process vstest.console, raw messages are passed to the translation layer but
+            // they are never read and don't get passed to the actual events handler in TW. If they
+            // were (as it happens for in-process vstest.console since there is no more translation
+            // layer) a NotImplemented exception would be raised as per the time this of writing this
+            // note.
+            //
+            // Consider changing this logic in the future if TW changes the handling logic for raw
+            // messages.
+        };
         _handleTestRunComplete = (_, e) => _handler.HandleTestRunComplete(e, null, null, null);
         _handleTestRunStatsChange = (_, e) => _handler.HandleTestRunStatsChange(e);
     }

src/vstest.console/InProcessVsTestConsoleWrapper.cs

@@ -16,11 +16,13 @@
 using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Helpers;
 using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Tracing;
 using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Tracing.Interfaces;
+using Microsoft.VisualStudio.TestPlatform.Execution;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.Interfaces;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.Payloads;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
+using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions;
 using Microsoft.VisualStudio.TestPlatform.Utilities;
 using Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces;
 using Microsoft.VisualStudio.TestPlatform.VsTestConsole.TranslationLayer.Interfaces;
@@ -52,7 +54,8 @@ public InProcessVsTestConsoleWrapper(ConsoleParameters consoleParameters)
               requestSender: new VsTestConsoleRequestSender(),
               testRequestManager: null,
               executor: new Executor(ConsoleOutput.Instance),
-              testPlatformEventSource: TestPlatformEventSource.Instance)
+              testPlatformEventSource: TestPlatformEventSource.Instance,
+              new())
     { }
 
     internal InProcessVsTestConsoleWrapper(
@@ -62,7 +65,29 @@ internal InProcessVsTestConsoleWrapper(
         ITestRequestManager? testRequestManager,
         Executor executor,
         ITestPlatformEventSource testPlatformEventSource)
+        : this(
+              consoleParameters,
+              environmentVariableHelper,
+              requestSender,
+              testRequestManager,
+              executor,
+              testPlatformEventSource,
+              new())
+    { }
+
+    internal InProcessVsTestConsoleWrapper(
+        ConsoleParameters consoleParameters,
+        IEnvironmentVariableHelper environmentVariableHelper,
+        ITranslationLayerRequestSender requestSender,
+        ITestRequestManager? testRequestManager,
+        Executor executor,
+        ITestPlatformEventSource testPlatformEventSource,
+        UiLanguageOverride languageOverride)
     {
+        // Setting the culture specified by user here since there's no more vstest.console process
+        // to set it for us. See vstest.console Main method for more info.
+        languageOverride.SetCultureSpecifiedByUser();
+
         EqtTrace.Info("VsTestConsoleWrapper.StartSession: Starting VsTestConsoleWrapper session.");
 
         _environmentVariableHelper = environmentVariableHelper;
@@ -89,9 +114,13 @@ internal InProcessVsTestConsoleWrapper(
         consoleParameters.PortNumber = port;
 
         // Start vstest.console.
-        // TODO: under VS we use consoleParameters.InheritEnvironmentVariables, we take that
-        // into account when starting a testhost, or clean up in the service host, and use the
-        // desired set, so all children can inherit it.
+        // Running vstest.console in process means we inherit all environment variables from the
+        // process we load the wrapper into. We do not want to alter this environment since that
+        // would mean we may interfer with the way the host process works. However, certain
+        // alterations are desired. The solution is to pass the environment variables we get via
+        // the console parameters directly to the testhost process and make sure that at least the
+        // testhost environment is predictable.
+        ProcessHelper.ExternalEnvironmentVariables = consoleParameters.EnvironmentVariables;
         foreach (var pair in consoleParameters.EnvironmentVariables)
         {
             if (pair.Value is null)
@@ -869,7 +898,7 @@ public async Task ProcessTestRunAttachmentsAsync(
                 await Task.Run(() =>
                         TestRequestManager?.ProcessTestRunAttachments(
                             attachmentProcessingPayload,
-                            eventsHandler,
+                            new InProcessTestRunAttachmentsProcessingEventsHandler(eventsHandler),
                             new ProtocolConfig { Version = _highestSupportedVersion }),
                         CancellationToken.None)
                     .ConfigureAwait(false);

test/vstest.console.UnitTests/InProcessVsTestConsoleWrapperTests.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -13,6 +14,7 @@
 using Microsoft.VisualStudio.TestPlatform.Client.RequestHelper;
 using Microsoft.VisualStudio.TestPlatform.Common.Interfaces;
 using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Tracing.Interfaces;
+using Microsoft.VisualStudio.TestPlatform.Execution;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client.Interfaces;
@@ -114,6 +116,55 @@ public void InProcessWrapperConstructorShouldSetEnvironmentVariablesReceivedAsCo
         _mockEnvironmentVariableHelper.Verify(evh => evh.SetEnvironmentVariable(environmentVariableName, "1"));
     }
 
+    [TestMethod]
+    public void InProcessWrapperConstructorShouldSetEnvironmentVariablesReceivedAsConsoleParametersForProcessHelper()
+    {
+        const string environmentVariableName = "AAAAA";
+
+        var consoleParams = new ConsoleParameters();
+        consoleParams.EnvironmentVariables.Add(environmentVariableName, "1");
+
+        var _ = new InProcessVsTestConsoleWrapper(
+            consoleParams,
+            _mockEnvironmentVariableHelper.Object,
+            _mockRequestSender.Object,
+            _mockTestRequestManager.Object,
+            new Executor(_mockOutput.Object, new Mock<ITestPlatformEventSource>().Object, new ProcessHelper(), new PlatformEnvironment()),
+            new Mock<ITestPlatformEventSource>().Object);
+
+        Assert.IsTrue(ProcessHelper.ExternalEnvironmentVariables?.ContainsKey(environmentVariableName));
+        Assert.IsTrue(ProcessHelper.ExternalEnvironmentVariables?[environmentVariableName] == "1");
+    }
+
+    [TestMethod]
+    public void InProcessWrapperConstructorShouldSetTheCultureSpecifiedByTheUser()
+    {
+        // Arrange
+        var culture = new CultureInfo("fr-fr");
+        _mockEnvironmentVariableHelper.Setup(x => x.GetEnvironmentVariable("DOTNET_CLI_UI_LANGUAGE")).Returns(culture.Name);
+
+        bool threadCultureWasSet = false;
+
+        // Act - We have an exception because we are not passing the right args but that's ok for our test
+        var consoleParams = new ConsoleParameters();
+        var _ = new InProcessVsTestConsoleWrapper(
+            consoleParams,
+            _mockEnvironmentVariableHelper.Object,
+            _mockRequestSender.Object,
+            _mockTestRequestManager.Object,
+            new Executor(_mockOutput.Object, new Mock<ITestPlatformEventSource>().Object, new ProcessHelper(), new PlatformEnvironment()),
+            new Mock<ITestPlatformEventSource>().Object,
+            new UiLanguageOverride(_mockEnvironmentVariableHelper.Object, lang => threadCultureWasSet = lang.Equals(culture)));
+
+        // Assert
+        Assert.IsTrue(threadCultureWasSet, "DefaultThreadCurrentUICulture was not set");
+        _mockEnvironmentVariableHelper.Verify(x => x.GetEnvironmentVariable("DOTNET_CLI_UI_LANGUAGE"), Times.Exactly(2));
+        _mockEnvironmentVariableHelper.Verify(x => x.GetEnvironmentVariable("VSLANG"), Times.Once);
+        _mockEnvironmentVariableHelper.Verify(x => x.SetEnvironmentVariable("VSLANG", culture.LCID.ToString(CultureInfo.InvariantCulture)), Times.Once);
+        _mockEnvironmentVariableHelper.Verify(x => x.GetEnvironmentVariable("PreferredUILang"), Times.Once);
+        _mockEnvironmentVariableHelper.Verify(x => x.SetEnvironmentVariable("PreferredUILang", culture.Name), Times.Once);
+    }
+
     [TestMethod]
     public void InProcessWrapperDiscoverTestsWithThreeParamsIsSuccessfullyInvoked()
     {