diff --git a/eng/Versions.props b/eng/Versions.props index 2365d69146..c563f7e964 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -84,6 +84,8 @@ 8.1.0 2.1.0 + + 2.4.0-preview.26410.1 4.16.1 18.3.0 diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpClientHelpers.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpClientHelpers.cs deleted file mode 100644 index 83ef3bdcef..0000000000 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpClientHelpers.cs +++ /dev/null @@ -1,97 +0,0 @@ -// 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.Diagnostics; - -using Jsonite; - -using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; - -namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; - -/// -/// Shared helpers for the MTP proxies. -/// -internal static class MtpClientHelpers -{ - public static Dictionary InitializeParameters() - => new() - { - ["processId"] = GetCurrentProcessId(), - ["clientInfo"] = new Dictionary - { - ["name"] = "vstest", - ["version"] = "1.0.0", - }, - ["capabilities"] = new Dictionary - { - ["testing"] = new Dictionary - { - ["debuggerProvider"] = false, - }, - }, - }; - - public static TestMessageLevel MapLevel(string level) - => level switch - { - "Error" or "Critical" => TestMessageLevel.Error, - "Warning" => TestMessageLevel.Warning, - _ => TestMessageLevel.Informational, - }; - - public static TimeSpan GetConnectionTimeout() - { - // Reuse vstest's connection timeout knob so users can extend it in slow environments. - string? value = Environment.GetEnvironmentVariable("VSTEST_CONNECTION_TIMEOUT"); - if (!string.IsNullOrEmpty(value) && int.TryParse(value, out int seconds) && seconds > 0) - { - return TimeSpan.FromSeconds(seconds); - } - - return TimeSpan.FromSeconds(90); - } - - private static int GetCurrentProcessId() - { - using var process = Process.GetCurrentProcess(); - return process.Id; - } - - /// - /// Returns true when a testing/testUpdates/tests notification is the completion sentinel - /// (its changes array is null or absent). - /// - public static bool IsCompletionSentinel(object? parameters) - { - JsonObject? node = MtpJson.AsObject(parameters); - return node is null - || !node.TryGetValue(MtpConstants.ChangesProperty, out object? changes) - || changes is null; - } - - /// - /// Enumerates the node objects carried by a testing/testUpdates/tests notification. - /// - public static IEnumerable EnumerateNodes(object? parameters) - { - if (MtpJson.AsObject(parameters) is not JsonObject node - || !node.TryGetValue(MtpConstants.ChangesProperty, out object? changesValue) - || changesValue is not JsonArray changes) - { - yield break; - } - - foreach (object? changeObject in changes) - { - if (changeObject is JsonObject change - && change.TryGetValue(MtpConstants.NodeProperty, out object? nodeValue) - && nodeValue is JsonObject testNode) - { - yield return testNode; - } - } - } -} diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpClientOptionsFactory.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpClientOptionsFactory.cs new file mode 100644 index 0000000000..f22df6af07 --- /dev/null +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpClientOptionsFactory.cs @@ -0,0 +1,88 @@ +// 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.Testing.Platform.ServerMode.Client; +using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Helpers; +using Microsoft.VisualStudio.TestPlatform.ObjectModel; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; + +namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; + +/// +/// vstest-side glue for the source-only Microsoft.Testing.Platform (MTP) server client: builds the +/// that identify vstest to the MTP application, bridge the client's +/// own diagnostics to , and map the server's client/log levels onto vstest's +/// . +/// +internal static class MtpClientOptionsFactory +{ + /// + /// Builds the options used to launch an MTP application: vstest's client identity, a single + /// discover-or-run session (not stateful), the vstest connection timeout, the EqtTrace diagnostics + /// bridge, and any environment variables to inject into the launched process. + /// + public static MtpServerClientOptions CreateOptions(IDictionary? environmentVariables = null) + { + var options = new MtpServerClientOptions + { + ClientName = "vstest", + ClientVersion = "1.0.0", + DebuggerProvider = false, + IsStateful = false, + ConnectionTimeout = GetConnectionTimeout(), + Logger = new DelegateMtpClientLogger(Trace), + }; + + if (environmentVariables is not null) + { + foreach (KeyValuePair variable in environmentVariables) + { + options.EnvironmentVariables[variable.Key] = variable.Value; + } + } + + return options; + } + + /// + /// Maps a server client/log level string onto the vstest . + /// + public static TestMessageLevel MapServerLogLevel(string level) + => level switch + { + "Error" or "Critical" => TestMessageLevel.Error, + "Warning" => TestMessageLevel.Warning, + _ => TestMessageLevel.Informational, + }; + + private static TimeSpan GetConnectionTimeout() + // Reuse vstest's shared connection-timeout knob (VSTEST_CONNECTION_TIMEOUT) rather than + // re-reading the environment variable here, so the MTP path honours exactly the same + // override, default and diagnostics as every other vstest connection. + => TimeSpan.FromSeconds(EnvironmentHelper.GetConnectionTimeout()); + + private static void Trace(MtpClientLogLevel level, string message) + { + switch (level) + { + case MtpClientLogLevel.Error: + EqtTrace.Error(message); + break; + + case MtpClientLogLevel.Warning: + EqtTrace.Warning(message); + break; + + case MtpClientLogLevel.Information: + EqtTrace.Info(message); + break; + + default: + EqtTrace.Verbose(message); + break; + } + } +} diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpConstants.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpConstants.cs deleted file mode 100644 index 0e43f0a5ca..0000000000 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpConstants.cs +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; - -/// -/// Constants for the Microsoft.Testing.Platform (MTP) server-mode JSON-RPC protocol. -/// See the MTP protocol docs in microsoft/testfx (ServerMode/JsonRpc). -/// -internal static class MtpConstants -{ - // Command line used to start an MTP application in JSON-RPC server mode. vstest opens a TCP - // listener and the application connects back to it (the application dials out to us). - public const string ServerArgument = "--server"; - public const string ClientPortArgument = "--client-port"; - public const string NoBannerArgument = "--no-banner"; - - // JSON-RPC method names. - public const string InitializeMethod = "initialize"; - public const string DiscoverTestsMethod = "testing/discoverTests"; - public const string RunTestsMethod = "testing/runTests"; - public const string TestUpdatesTestsMethod = "testing/testUpdates/tests"; - public const string TestUpdatesAttachmentsMethod = "testing/testUpdates/attachments"; - public const string ClientLogMethod = "client/log"; - public const string ExitMethod = "exit"; - - // Framing (LSP-like headers). - public const string ContentLengthHeader = "Content-Length:"; - public const string ContentType = "application/testingplatform"; - - // Request/notification parameter keys. - public const string RunIdParameter = "runId"; - public const string TestsParameter = "tests"; - public const string ChangesProperty = "changes"; - public const string NodeProperty = "node"; - public const string AttachmentsProperty = "attachments"; - public const string AttachmentUriProperty = "uri"; - public const string AttachmentPathProperty = "path"; - - // TestNode wire property keys (pure MTP shape). - public const string Uid = "uid"; - public const string DisplayName = "display-name"; - public const string NodeType = "node-type"; - public const string ExecutionState = "execution-state"; - public const string TimeDurationMs = "time.duration-ms"; - public const string ErrorMessage = "error.message"; - public const string ErrorStackTrace = "error.stacktrace"; - public const string StandardOutput = "standardOutput"; - public const string StandardError = "standardError"; - public const string LocationFile = "location.file"; - public const string LocationLineStart = "location.line-start"; - public const string Traits = "traits"; - - // Execution states. - public const string StateDiscovered = "discovered"; - public const string StateInProgress = "in-progress"; - public const string StatePassed = "passed"; - public const string StateSkipped = "skipped"; - public const string StateFailed = "failed"; - public const string StateError = "error"; - public const string StateTimedOut = "timed-out"; - public const string StateCanceled = "canceled"; - - // Optional VSTest-provider properties (present only when the app still runs on the VSTestBridge). - // The converter treats these as best-effort enrichment and never requires them, so that a pure - // MTP app with no vstest dependency at all still converts correctly. - public const string VsTestFullyQualifiedName = "vstest.TestCase.FullyQualifiedName"; - public const string VsTestId = "vstest.TestCase.Id"; - public const string VsTestExecutorUri = "vstest.original-executor-uri"; - - // Synthetic executor URI used when the app does not expose the vstest provider properties. - public const string DefaultExecutorUri = "executor://MicrosoftTestingPlatform/v1"; - - // Property used to round-trip the MTP node uid on a vstest TestCase so we can request a - // filtered run by uid after discovery. - public const string MtpUidPropertyId = "MTP.TestNode.Uid"; -} diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpJson.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpJson.cs deleted file mode 100644 index 038a77b397..0000000000 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpJson.cs +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using Jsonite; - -namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; - -/// -/// Small accessors over the Jsonite JSON object model used by the MTP JSON-RPC client. -/// -/// The MTP wire is serialized with Jsonite (not System.Text.Json) so that the client works on every -/// framework we ship — including the .NET Framework runner, where taking a dependency on -/// System.Text.Json would introduce binding-redirect fallout in hosts that run without them. Parsed -/// JSON is a plain object graph: objects are (a -/// Dictionary<string, object>), arrays are (a -/// List<object>), numbers are int/long/double, and everything else -/// is string/bool/null. -/// -internal static class MtpJson -{ - public static JsonObject? AsObject(object? node) => node as JsonObject; - - public static JsonArray? AsArray(object? node) => node as JsonArray; - - public static object? GetValue(JsonObject? node, string key) - => node is not null && node.TryGetValue(key, out object? value) ? value : null; - - public static string? GetString(JsonObject? node, string key) - => GetValue(node, key) as string; - - public static bool TryGetInt(JsonObject? node, string key, out int result) - => TryToInt(GetValue(node, key), out result); - - public static bool TryGetDouble(JsonObject? node, string key, out double result) - { - switch (GetValue(node, key)) - { - case double d: result = d; return true; - case int i: result = i; return true; - case long l: result = l; return true; - case decimal m: result = (double)m; return true; - default: result = 0; return false; - } - } - - public static bool TryToInt(object? value, out int result) - { - switch (value) - { - case int i: result = i; return true; - case long l: result = unchecked((int)l); return true; - case double d: result = (int)d; return true; - case decimal m: result = (int)m; return true; - default: result = 0; return false; - } - } -} diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyDiscoveryManager.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyDiscoveryManager.cs index 07a36ed168..5d9e5f00f2 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyDiscoveryManager.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyDiscoveryManager.cs @@ -6,8 +6,7 @@ using System.Linq; using System.Threading; -using Jsonite; - +using Microsoft.Testing.Platform.ServerMode.Client; using Microsoft.VisualStudio.TestPlatform.ObjectModel; using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client; using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine; @@ -85,44 +84,37 @@ public void Dispose() private int DiscoverSource(string source, ITestDiscoveryEventsHandler2 eventHandler) { var discovered = new List(); - var completed = new ManualResetEventSlim(false); - using var connection = new MtpServerConnection(); - connection.LogReceived += (level, message) => eventHandler.HandleLogMessage(MtpClientHelpers.MapLevel(level), message); - connection.TestNodesUpdated += parameters => + MtpServerClientOptions options = MtpClientOptionsFactory.CreateOptions(); + using IMtpServerClient client = MtpServerClientFactory.Launch(source, options); + client.LogReceived += (_, e) => eventHandler.HandleLogMessage(MtpClientOptionsFactory.MapServerLogLevel(e.Level), e.Message); + client.TestNodesUpdated += (_, e) => { - if (MtpClientHelpers.IsCompletionSentinel(parameters)) + foreach (MtpTestNodeUpdate change in e.Changes) { - completed.Set(); - return; - } - - foreach (JsonObject node in MtpClientHelpers.EnumerateNodes(parameters)) - { - if (MtpTestNodeConverter.IsActionNode(node)) + if (MtpTestNodeConverter.IsActionNode(change)) { lock (discovered) { - discovered.Add(MtpTestNodeConverter.ToTestCase(node, source)); + discovered.Add(MtpTestNodeConverter.ToTestCase(change, source)); } } } }; - connection.Start(source, environmentVariables: null, MtpClientHelpers.GetConnectionTimeout()); - connection.InvokeAsync(MtpConstants.InitializeMethod, MtpClientHelpers.InitializeParameters(), _cancellationTokenSource.Token).GetAwaiter().GetResult(); - - var runId = Guid.NewGuid(); - var discoverTask = connection.InvokeAsync( - MtpConstants.DiscoverTestsMethod, - new Dictionary { [MtpConstants.RunIdParameter] = runId.ToString() }, - _cancellationTokenSource.Token); + try + { + client.InitializeAsync(_cancellationTokenSource.Token).GetAwaiter().GetResult(); - // The response indicates the server has finished discovery. Because messages arrive on a - // single ordered stream that we read sequentially, every node notification sent before the - // response has already been dispatched by the time the response completes. - discoverTask.GetAwaiter().GetResult(); - completed.Wait(TimeSpan.FromSeconds(3)); + // Awaiting the discover request is sufficient: server-to-client messages arrive on a single + // ordered stream that the client reads sequentially and dispatches synchronously, so every + // node notification has already been delivered by the time the request completes. + client.DiscoverTestsAsync(_cancellationTokenSource.Token).GetAwaiter().GetResult(); + } + finally + { + MtpServerClientFactory.TryExit(client); + } List chunk; lock (discovered) @@ -135,7 +127,6 @@ private int DiscoverSource(string source, ITestDiscoveryEventsHandler2 eventHand eventHandler.HandleDiscoveredTests(chunk); } - connection.SendNotification(MtpConstants.ExitMethod, null); return chunk.Count; } } diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyExecutionManager.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyExecutionManager.cs index 0fe50c394e..6da0a1dd90 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyExecutionManager.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyExecutionManager.cs @@ -4,11 +4,12 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Globalization; using System.IO; using System.Linq; using System.Threading; -using Jsonite; +using Microsoft.Testing.Platform.ServerMode.Client; using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client; using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.DataCollection; @@ -18,6 +19,8 @@ using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine; using Microsoft.VisualStudio.TestPlatform.Utilities; +using CrossPlatEngineResources = Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Resources.Resources; + namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; /// @@ -169,7 +172,7 @@ private void BeforeTestRun(IInternalTestRunEventsHandler eventHandler) if (parameters?.EnvironmentVariables is { } dataCollectionEnvironmentVariables) { - EnvironmentVariables ??= new Dictionary(); + EnvironmentVariables ??= CreateEnvironmentVariablesDictionary(); foreach (KeyValuePair variable in dataCollectionEnvironmentVariables) { EnvironmentVariables[variable.Key] = variable.Value; @@ -311,39 +314,32 @@ private int RunSource( List attachments, HashSet executorUris) { - var completed = new ManualResetEventSlim(false); - - using var connection = new MtpServerConnection(); - connection.LogReceived += (level, message) => eventHandler.HandleLogMessage(MtpClientHelpers.MapLevel(level), message); - connection.TestNodesUpdated += parameters => + MtpServerClientOptions options = MtpClientOptionsFactory.CreateOptions(EnvironmentVariables); + using IMtpServerClient client = MtpServerClientFactory.Launch(source, options); + client.LogReceived += (_, e) => eventHandler.HandleLogMessage(MtpClientOptionsFactory.MapServerLogLevel(e.Level), e.Message); + client.TestNodesUpdated += (_, e) => { - if (MtpClientHelpers.IsCompletionSentinel(parameters)) - { - completed.Set(); - return; - } - var results = new List(); - foreach (JsonObject node in MtpClientHelpers.EnumerateNodes(parameters)) + foreach (MtpTestNodeUpdate change in e.Changes) { - if (!MtpTestNodeConverter.IsActionNode(node)) + if (!MtpTestNodeConverter.IsActionNode(change)) { continue; } - string? state = MtpTestNodeConverter.GetExecutionState(node); + string? state = change.ExecutionState; if (EqtTrace.IsVerboseEnabled) { - EqtTrace.Verbose("MtpProxyExecutionManager: node update uid={0} state={1}", MtpJson.GetString(node, MtpConstants.Uid), state ?? "(none)"); + EqtTrace.Verbose("MtpProxyExecutionManager: node update uid={0} state={1}", change.Uid, state ?? "(none)"); } // A test entering the in-progress state is our "test started" signal. Forwarding it // lets per-test-case collectors (e.g. Blame) know which test is in flight, which is // what makes crash attribution work when the test never reaches a terminal state. - if (_testCaseEventForwarder is { } forwarder && state == MtpConstants.StateInProgress) + if (_testCaseEventForwarder is { } forwarder && MtpTestNodeConverter.IsInProgressState(state)) { - forwarder.NotifyTestCaseStart(MtpTestNodeConverter.ToTestCase(node, source)); + forwarder.NotifyTestCaseStart(MtpTestNodeConverter.ToTestCase(change, source)); continue; } @@ -352,7 +348,7 @@ private int RunSource( continue; } - TestResult result = MtpTestNodeConverter.ToTestResult(node, source); + TestResult result = MtpTestNodeConverter.ToTestResult(change, source); _testCaseEventForwarder?.NotifyTestCaseEnd(result); results.Add(result); } @@ -380,28 +376,31 @@ private int RunSource( eventHandler.HandleTestRunStatsChange(new TestRunChangedEventArgs(snapshot, results, null)); }; - connection.Start(source, EnvironmentVariables, MtpClientHelpers.GetConnectionTimeout()); - // Let the data collector (e.g. code coverage) know the process it should track. The profiler - // env vars were already injected via EnvironmentVariables above. - _dataCollectionManager?.TestHostLaunched(connection.ProcessId); + // env vars were already injected via the launch options above. Capture the id here rather than + // reading it again after the exit handshake, when the process may already be gone. + int processId = client.ProcessId; + _dataCollectionManager?.TestHostLaunched(processId); - connection.InvokeAsync(MtpConstants.InitializeMethod, MtpClientHelpers.InitializeParameters(), _cancellationTokenSource.Token).GetAwaiter().GetResult(); + try + { + client.InitializeAsync(_cancellationTokenSource.Token).GetAwaiter().GetResult(); + + // Awaiting the run request is sufficient: server-to-client messages arrive on a single ordered + // stream that the client reads sequentially and dispatches synchronously, so every node update + // has already been delivered by the time the request completes. + MtpRunResult runResult = (tests is { Count: > 0 } + ? client.RunTestsAsync(BuildUids(tests), _cancellationTokenSource.Token) + : client.RunTestsAsync(_cancellationTokenSource.Token)).GetAwaiter().GetResult(); - var runId = Guid.NewGuid(); - var runParameters = new Dictionary { [MtpConstants.RunIdParameter] = runId.ToString() }; - if (tests is { Count: > 0 }) + CollectAttachments(runResult, attachments); + } + finally { - runParameters[MtpConstants.TestsParameter] = BuildTestsFilter(tests); + MtpServerClientFactory.TryExit(client); } - var runTask = connection.InvokeAsync(MtpConstants.RunTestsMethod, runParameters, _cancellationTokenSource.Token); - object? response = runTask.GetAwaiter().GetResult(); - completed.Wait(TimeSpan.FromSeconds(3)); - - CollectAttachments(response, attachments); - connection.SendNotification(MtpConstants.ExitMethod, null); - return connection.ProcessId; + return processId; } private static IEnumerable<(string Source, List? Tests)> BuildWork(TestRunCriteria criteria) @@ -430,42 +429,74 @@ private void ApplyRunSettingsEnvironmentVariables(string? runSettings) return; } - EnvironmentVariables ??= new Dictionary( - Environment.OSVersion.Platform == PlatformID.Win32NT ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); + EnvironmentVariables ??= CreateEnvironmentVariablesDictionary(); foreach (KeyValuePair variable in runSettingsEnvironmentVariables) { EnvironmentVariables[variable.Key] = variable.Value; } } - private static List> BuildTestsFilter(List tests) - => tests - .Select(test => new Dictionary + /// + /// Creates the dictionary used to collect environment variables for the MTP application launch, + /// keyed case-insensitively on Windows (matching the classic testhost path) and case-sensitively + /// elsewhere, so callers that pass case-variant duplicate keys collapse the same way the classic + /// path did before the values reach the ordinal-keyed + /// . + /// + private static Dictionary CreateEnvironmentVariablesDictionary() + => new(Environment.OSVersion.Platform == PlatformID.Win32NT ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); + + /// + /// Projects the tests selected for a filtered run onto the MTP node uids the server matches on. + /// + /// + /// A selected test carries no MTP node uid, so the run cannot be expressed. + /// + private static IReadOnlyCollection BuildUids(List tests) + { + var uids = new List(tests.Count); + foreach (TestCase test in tests) + { + string? uid = test.GetPropertyValue(MtpTestNodeConverter.MtpUidProperty, null); + + // The MTP server projects node.Uid alone when it builds a run filter and never reads + // DisplayName or any other field, so a TestCase without MTP.TestNode.Uid simply cannot be + // addressed. Substituting FullyQualifiedName here (as this method previously did) produces + // a filter the server matches nothing against: the run completes "successfully" having + // executed zero of the tests the user selected, with no error anywhere. Failing here turns + // that invisible wrong answer into a visible, actionable one. Do not reintroduce a + // fallback - there is no value that works other than the uid the server itself issued. + // + // This aborts the whole source rather than skipping the offending test: the caller reports + // the failure and marks the run aborted, which is deliberate. Silently running the + // addressable subset would recreate the same class of bug in a smaller form, reporting a + // partial run as if it were the run the user asked for. + if (uid.IsNullOrEmpty()) { - [MtpConstants.Uid] = test.GetPropertyValue(MtpTestNodeConverter.MtpUidProperty, test.FullyQualifiedName), - [MtpConstants.DisplayName] = test.DisplayName, - }) - .ToList(); + throw new TestPlatformException( + string.Format( + CultureInfo.CurrentCulture, + CrossPlatEngineResources.MtpTestCaseMissingNodeUid, + test.DisplayName ?? test.FullyQualifiedName)); + } + + uids.Add(uid); + } - private static void CollectAttachments(object? response, List attachments) + return uids; + } + + private static void CollectAttachments(MtpRunResult runResult, List attachments) { - if (MtpJson.AsObject(response) is not JsonObject responseObject - || !responseObject.TryGetValue(MtpConstants.AttachmentsProperty, out object? attachmentsValue) - || attachmentsValue is not JsonArray attachmentArray) + if (runResult.Artifacts.Count == 0) { return; } - var set = new AttachmentSet(new Uri(MtpConstants.DefaultExecutorUri), "Microsoft.Testing.Platform"); - foreach (object? attachmentObject in attachmentArray) + var set = new AttachmentSet(new Uri(MtpTestNodeConverter.DefaultExecutorUri), "Microsoft.Testing.Platform"); + foreach (MtpAttachment artifact in runResult.Artifacts) { - if (attachmentObject is not JsonObject attachment) - { - continue; - } - - string? path = GetStringProperty(attachment, MtpConstants.AttachmentUriProperty) - ?? GetStringProperty(attachment, MtpConstants.AttachmentPathProperty); + string? path = artifact.Uri; if (string.IsNullOrEmpty(path)) { continue; @@ -476,7 +507,7 @@ private static void CollectAttachments(object? response, List att continue; } - string display = GetStringProperty(attachment, MtpConstants.DisplayName) ?? Path.GetFileName(path!); + string display = artifact.DisplayName ?? Path.GetFileName(path!); set.Attachments.Add(new UriDataAttachment(fileUri!, display)); } @@ -489,9 +520,6 @@ private static void CollectAttachments(object? response, List att } } - private static string? GetStringProperty(JsonObject element, string name) - => element.TryGetValue(name, out object? value) && value is string text ? text : null; - private static bool TryCreateFileUri(string path, out Uri? uri) { try diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpServerClientFactory.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpServerClientFactory.cs new file mode 100644 index 0000000000..25fe1ae120 --- /dev/null +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpServerClientFactory.cs @@ -0,0 +1,58 @@ +// 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.Threading; + +using Microsoft.Testing.Platform.ServerMode.Client; +using Microsoft.VisualStudio.TestPlatform.ObjectModel; + +namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; + +/// +/// Creates the the MTP proxy managers drive, and shuts it down. +/// +/// The launch is behind a replaceable delegate purely so the proxy managers can be unit tested +/// without starting a real test application; production code always uses +/// . +/// +internal static class MtpServerClientFactory +{ + /// + /// How long to wait for the server to acknowledge exit before abandoning the graceful + /// shutdown and falling back to disposing the client (which terminates the process). + /// + private static readonly TimeSpan ExitTimeout = TimeSpan.FromSeconds(10); + + /// + /// Launches an MTP application in server mode. Replaceable for testing only. + /// + internal static Func Launch { get; set; } + = static (source, options) => MtpServerClient.Launch(source, options); + + /// + /// Asks the server to exit, on a best-effort basis. + /// + /// Two things matter here and neither is served by passing the run's own cancellation token. + /// First, shutdown must still happen when the run was cancelled or aborted - that is precisely + /// when the token is already cancelled, so using it would make exit throw immediately and + /// skip the handshake entirely. Second, exit is a request/response call rather than the + /// fire-and-forget notification it replaced, so an unresponsive test application would otherwise + /// block the run forever; it is bounded here instead. + /// + /// Failing to exit cleanly is never fatal: the caller disposes the client afterwards, which + /// tears the process down regardless. + /// + internal static void TryExit(IMtpServerClient client) + { + try + { + using var timeout = new CancellationTokenSource(ExitTimeout); + client.ExitAsync(timeout.Token).GetAwaiter().GetResult(); + } + catch (Exception ex) + { + EqtTrace.Warning("MtpServerClientFactory.TryExit: graceful exit failed, disposing instead. {0}", ex); + } + } +} diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpServerConnection.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpServerConnection.cs deleted file mode 100644 index 8f2d7a0781..0000000000 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpServerConnection.cs +++ /dev/null @@ -1,532 +0,0 @@ -// 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.Concurrent; -using System.Collections.Generic; -using System.Diagnostics; -using System.IO; -using System.Net; -using System.Net.Sockets; -using System.Runtime.InteropServices; -using System.Text; -using System.Threading; -using System.Threading.Tasks; - -using Jsonite; - -using Microsoft.VisualStudio.TestPlatform.ObjectModel; - -namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; - -/// -/// Manages a single Microsoft.Testing.Platform (MTP) application running in JSON-RPC server mode. -/// -/// vstest is the JSON-RPC client here: it opens a loopback TCP listener, launches the MTP -/// application with --server --client-port <port>, and the application connects back to -/// the listener. Messages are framed with LSP-style Content-Length headers. -/// -/// The wire is serialized with Jsonite rather than System.Text.Json so this client works on every -/// framework we ship, including the .NET Framework runner (no System.Text.Json dependency and thus -/// no binding-redirect fallout). Parsed messages are plain / object graphs. -/// -internal sealed class MtpServerConnection : IDisposable -{ - private readonly TcpListener _listener; - private readonly int _port; - private readonly ConcurrentDictionary> _pending = new(); - private readonly object _writeLock = new(); - private readonly CancellationTokenSource _cts = new(); - private readonly StringBuilder _standardError = new(); - - private TcpClient? _client; - private Stream? _stream; - private Process? _process; - private Task? _readLoop; - private int _nextId; - private bool _disposed; - - /// - /// Raised for each testing/testUpdates/tests notification. The argument is the parsed - /// notification params value (a ), or null when absent. - /// - public event Action? TestNodesUpdated; - - /// - /// Raised for each client/log notification with (level, message). - /// - public event Action? LogReceived; - - public MtpServerConnection() - { - _listener = new TcpListener(IPAddress.Loopback, 0); - _listener.Start(); - _port = ((IPEndPoint)_listener.LocalEndpoint).Port; - } - - /// - /// Gets the process id of the launched MTP application, or 0 if it has not been launched. - /// - public int ProcessId => _process?.Id ?? 0; - - /// - /// Launches the MTP application in server mode and waits for it to connect back. - /// - public void Start(string source, IDictionary? environmentVariables, TimeSpan connectionTimeout) - { - var (fileName, arguments, workingDirectory) = BuildLaunch(source, _port); - EqtTrace.Info("MtpServerConnection.Start: launching '{0} {1}' (cwd '{2}') listening on port {3}.", fileName, arguments, workingDirectory, _port); - - var startInfo = new ProcessStartInfo - { - FileName = fileName, - Arguments = arguments, - WorkingDirectory = workingDirectory, - UseShellExecute = false, - RedirectStandardError = true, - RedirectStandardOutput = true, - }; - - if (environmentVariables != null) - { - foreach (var kvp in environmentVariables) - { - startInfo.Environment[kvp.Key] = kvp.Value; - } - } - - _process = new Process { StartInfo = startInfo, EnableRaisingEvents = true }; - _process.ErrorDataReceived += (_, e) => - { - if (e.Data != null) - { - lock (_standardError) - { - _standardError.AppendLine(e.Data); - } - } - }; - _process.Start(); - _process.BeginErrorReadLine(); - // Drain stdout so the child never blocks on a full pipe (banner, diagnostics). - _process.BeginOutputReadLine(); - - var acceptTask = _listener.AcceptTcpClientAsync(); - if (!acceptTask.Wait(connectionTimeout)) - { - throw new TimeoutException($"The Microsoft.Testing.Platform application '{source}' did not connect back within {connectionTimeout.TotalSeconds:N0}s. {GetStandardError()}"); - } - - _client = acceptTask.GetAwaiter().GetResult(); - _client.NoDelay = true; - _stream = _client.GetStream(); - _readLoop = Task.Run(() => ReadLoopAsync(_cts.Token)); - } - - /// - /// Sends a JSON-RPC request and awaits the response. - /// - public async Task InvokeAsync(string method, object? parameters, CancellationToken cancellationToken) - { - int id = Interlocked.Increment(ref _nextId); - var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - _pending[id] = tcs; - - var envelope = new Dictionary - { - ["jsonrpc"] = "2.0", - ["id"] = id, - ["method"] = method, - ["params"] = parameters ?? new Dictionary(), - }; - - WriteMessage(envelope); - - using var registration = cancellationToken.Register(static state => ((TaskCompletionSource)state!).TrySetCanceled(), tcs); - try - { - return await tcs.Task.ConfigureAwait(false); - } - finally - { - _pending.TryRemove(id, out _); - } - } - - /// - /// Sends a JSON-RPC notification (no response expected). - /// - public void SendNotification(string method, object? parameters) - { - var envelope = new Dictionary - { - ["jsonrpc"] = "2.0", - ["method"] = method, - ["params"] = parameters ?? new Dictionary(), - }; - WriteMessage(envelope); - } - - public string GetStandardError() - { - lock (_standardError) - { - var text = _standardError.ToString().Trim(); - return text.Length == 0 ? string.Empty : $"Standard error: {text}"; - } - } - - private void WriteMessage(Dictionary envelope) - { - if (_stream is null) - { - throw new InvalidOperationException("MTP connection has not been established."); - } - - string json = Json.Serialize(envelope); - byte[] body = Encoding.UTF8.GetBytes(json); - byte[] header = Encoding.ASCII.GetBytes($"{MtpConstants.ContentLengthHeader} {body.Length}\r\nContent-Type: {MtpConstants.ContentType}\r\n\r\n"); - - lock (_writeLock) - { - _stream.Write(header, 0, header.Length); - _stream.Write(body, 0, body.Length); - _stream.Flush(); - } - } - - private async Task ReadLoopAsync(CancellationToken cancellationToken) - { - Debug.Assert(_stream != null, "Stream must be set before the read loop starts."); - try - { - while (!cancellationToken.IsCancellationRequested) - { - int contentLength = await ReadHeadersAsync(_stream!, cancellationToken).ConfigureAwait(false); - if (contentLength < 0) - { - break; // stream closed - } - - byte[] body = await ReadExactlyAsync(_stream!, contentLength, cancellationToken).ConfigureAwait(false); - if (body.Length < contentLength) - { - break; // stream closed mid-message - } - - Dispatch(body); - } - } - catch (OperationCanceledException) - { - } - catch (Exception ex) when (ex is IOException or ObjectDisposedException or SocketException) - { - // Connection closed; fail any pending requests below. - } - catch (Exception ex) - { - EqtTrace.Error("MtpServerConnection.ReadLoopAsync: unexpected error: {0}", ex); - } - finally - { - FailPending(new IOException($"The MTP connection was closed. {GetStandardError()}")); - } - } - - private void Dispatch(byte[] body) - { - object? parsed; - try - { - parsed = Json.Deserialize(Encoding.UTF8.GetString(body)); - } - catch (JsonException ex) - { - EqtTrace.Error("MtpServerConnection.Dispatch: failed to parse message: {0}", ex); - return; - } - - if (parsed is not JsonObject root) - { - return; - } - - int? id = root.TryGetValue("id", out object? idValue) && MtpJson.TryToInt(idValue, out int parsedId) - ? parsedId - : (int?)null; - - if (root.TryGetValue("method", out object? methodValue) && methodValue is string method) - { - object? parameters = root.TryGetValue("params", out object? p) ? p : null; - HandleServerMessage(method, parameters, id); - return; - } - - if (id is int responseId && _pending.TryGetValue(responseId, out var tcs)) - { - if (root.TryGetValue("error", out object? errorValue) && errorValue is JsonObject error) - { - string message = error.TryGetValue("message", out object? m) && m is string ms ? ms : "unknown error"; - tcs.TrySetException(new InvalidOperationException($"MTP request '{responseId}' failed: {message}")); - } - else - { - object? result = root.TryGetValue("result", out object? r) ? r : null; - tcs.TrySetResult(result); - } - } - } - - private void HandleServerMessage(string method, object? parameters, int? id) - { - switch (method) - { - case MtpConstants.TestUpdatesTestsMethod: - TestNodesUpdated?.Invoke(parameters); - break; - - case MtpConstants.ClientLogMethod: - JsonObject? logParams = MtpJson.AsObject(parameters); - string level = MtpJson.GetString(logParams, "level") ?? "Information"; - string message = MtpJson.GetString(logParams, "message") ?? string.Empty; - LogReceived?.Invoke(level, message); - break; - - default: - // Requests from the server (e.g. client/attachDebugger, client/launchDebugger) must be - // answered so the server does not block. We never request debugging, so decline. - if (id.HasValue) - { - var response = new Dictionary - { - ["jsonrpc"] = "2.0", - ["id"] = id.Value, - ["result"] = new Dictionary { ["success"] = false }, - }; - try - { - WriteMessage(response); - } - catch (Exception ex) - { - EqtTrace.Warning("MtpServerConnection.HandleServerMessage: failed to answer '{0}': {1}", method, ex); - } - } - - break; - } - } - - private static async Task ReadHeadersAsync(Stream stream, CancellationToken cancellationToken) - { - int contentLength = -1; - while (true) - { - string? line = await ReadAsciiLineAsync(stream, cancellationToken).ConfigureAwait(false); - if (line is null) - { - return -1; // stream closed - } - - if (line.Length == 0) - { - return contentLength; // blank line terminates headers - } - - if (line.StartsWith(MtpConstants.ContentLengthHeader, StringComparison.OrdinalIgnoreCase)) - { - _ = int.TryParse(line.Substring(MtpConstants.ContentLengthHeader.Length).Trim(), out contentLength); - } - } - } - - private static async Task ReadAsciiLineAsync(Stream stream, CancellationToken cancellationToken) - { - var bytes = new List(64); - var one = new byte[1]; - while (true) - { - int read = await stream.ReadAsync(one, 0, 1, cancellationToken).ConfigureAwait(false); - if (read == 0) - { - return bytes.Count == 0 ? null : Encoding.ASCII.GetString(bytes.ToArray()); - } - - if (one[0] == (byte)'\n') - { - if (bytes.Count > 0 && bytes[bytes.Count - 1] == (byte)'\r') - { - bytes.RemoveAt(bytes.Count - 1); - } - - return Encoding.ASCII.GetString(bytes.ToArray()); - } - - bytes.Add(one[0]); - } - } - - private static async Task ReadExactlyAsync(Stream stream, int count, CancellationToken cancellationToken) - { - var buffer = new byte[count]; - int offset = 0; - while (offset < count) - { - int read = await stream.ReadAsync(buffer, offset, count - offset, cancellationToken).ConfigureAwait(false); - if (read == 0) - { - Array.Resize(ref buffer, offset); - break; - } - - offset += read; - } - - return buffer; - } - - private void FailPending(Exception exception) - { - foreach (var kvp in _pending) - { - kvp.Value.TrySetException(exception); - } - - _pending.Clear(); - } - - internal static (string fileName, string arguments, string workingDirectory) BuildLaunch(string source, int port) - { - string serverArgs = $"{MtpConstants.ServerArgument} {MtpConstants.ClientPortArgument} {port} {MtpConstants.NoBannerArgument}"; - string workingDirectory = Path.GetDirectoryName(source) ?? Directory.GetCurrentDirectory(); - string extension = Path.GetExtension(source); - - if (extension.Equals(".exe", StringComparison.OrdinalIgnoreCase)) - { - return (source, serverArgs, workingDirectory); - } - - // A .NET MTP app is typically shipped as a dll with a sibling native apphost. Prefer the - // apphost if it is a usable executable for this platform, otherwise fall back to - // `dotnet `. The apphost file name is OS-specific: `Foo.exe` on Windows, but an - // extension-less `Foo` on Unix. Probing for `.exe` unconditionally is wrong on Unix: a - // payload built on Windows and run on Unix (for example tests built on a Windows agent and - // executed on a Linux Helix machine) can drag a Windows PE `Foo.exe` next to the dll. - // Launching that yields "Permission denied" (or "Exec format error" once it is +x), so the - // probe must be OS-aware and, on Unix, insist the candidate is actually executable. -#if NETFRAMEWORK - // .NET Framework only runs on Windows, where the apphost is .exe. - string apphost = Path.ChangeExtension(source, ".exe"); -#else - string apphost = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) - ? Path.ChangeExtension(source, ".exe") - : Path.ChangeExtension(source, null); -#endif - - return IsUsableApphost(apphost) - ? (apphost, serverArgs, workingDirectory) - : ("dotnet", $"\"{source}\" {serverArgs}", workingDirectory); - } - - /// - /// Determines whether can be launched directly as a native executable - /// on the current platform. On Windows, presence is sufficient. On Unix, when built for a target - /// framework that exposes File.GetUnixFileMode (.NET 7+), the file must also - /// carry an execute bit; a file that merely exists (for example a Windows PE copied onto Unix) is - /// not a usable apphost and the caller should fall back to dotnet <dll>. On target - /// frameworks without that API (e.g. netstandard2.0) this degrades to an existence-only check — - /// the OS-aware probe in already avoids the Windows-.exe-on-Unix - /// case, so the execute-bit check is defence-in-depth rather than the primary guard. - /// - internal static bool IsUsableApphost(string apphost) - { - if (!File.Exists(apphost)) - { - return false; - } - -#if NET - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - const UnixFileMode executeBits = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; - return (File.GetUnixFileMode(apphost) & executeBits) != 0; - } -#endif - return true; - } - - public void Dispose() - { - if (_disposed) - { - return; - } - - _disposed = true; - - try - { - _cts.Cancel(); - } - catch - { - // ignore - } - - try - { - _stream?.Dispose(); - } - catch - { - // ignore - } - - try - { - _client?.Dispose(); - } - catch - { - // ignore - } - - try - { - _listener.Stop(); - } - catch - { - // ignore - } - - try - { - if (_process is { HasExited: false }) - { -#if NETCOREAPP - _process.Kill(entireProcessTree: true); -#else - _process.Kill(); -#endif - } - } - catch - { - // ignore - } - - try - { - _process?.Dispose(); - } - catch - { - // ignore - } - - _cts.Dispose(); - } -} diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpTestNodeConverter.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpTestNodeConverter.cs index d84b11c4d2..865f369681 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpTestNodeConverter.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpTestNodeConverter.cs @@ -3,26 +3,57 @@ using System; using System.Collections.Generic; +using System.Globalization; -using Jsonite; - +using Microsoft.Testing.Platform.ServerMode.Client; using Microsoft.VisualStudio.TestPlatform.ObjectModel; namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; /// -/// Converts Microsoft.Testing.Platform (MTP) test nodes into vstest ObjectModel +/// Converts Microsoft.Testing.Platform (MTP) test-node updates into vstest ObjectModel /// and instances. /// -/// The converter works purely off the MTP node shape (uid, display-name, -/// execution-state, time.duration-ms, error.*, location.*, traits) -/// so that an MTP application with no dependency on vstest at all still converts correctly. When the -/// optional vstest.* bridge properties are present they are used purely as enrichment. +/// The typed accessors cover the common fields (uid, display name, +/// node type, execution state, error, duration). The remaining MTP node fields that vstest surfaces +/// (standardOutput/standardError, location.*, traits, and the optional +/// vstest.* bridge properties) are read from the raw +/// property bag by key, so an MTP application with no dependency on vstest at all still converts +/// correctly and the bridge properties are used purely as enrichment when present. /// internal static class MtpTestNodeConverter { + // Property used to round-trip the MTP node uid on a vstest TestCase so we can request a + // filtered run by uid after discovery. + private const string MtpUidPropertyId = "MTP.TestNode.Uid"; + + // Synthetic executor URI used when the app does not expose the vstest provider properties. + internal const string DefaultExecutorUri = "executor://MicrosoftTestingPlatform/v1"; + + // Raw TestNode wire keys not covered by the package's typed accessors. + private const string StandardOutputKey = "standardOutput"; + private const string StandardErrorKey = "standardError"; + private const string LocationFileKey = "location.file"; + private const string LocationLineStartKey = "location.line-start"; + private const string TraitsKey = "traits"; + + // Optional VSTest-provider properties (present only when the app still runs on the VSTestBridge). + private const string VsTestFullyQualifiedNameKey = "vstest.TestCase.FullyQualifiedName"; + private const string VsTestExecutorUriKey = "vstest.original-executor-uri"; + + // Execution states (MTP wire values). + private const string StateInProgress = "in-progress"; + private const string StatePassed = "passed"; + private const string StateSkipped = "skipped"; + private const string StateFailed = "failed"; + private const string StateError = "error"; + private const string StateTimedOut = "timed-out"; + + // Node type of a runnable test (leaf) as opposed to a grouping node. + private const string ActionNodeType = "action"; + internal static readonly TestProperty MtpUidProperty = TestProperty.Register( - MtpConstants.MtpUidPropertyId, + MtpUidPropertyId, "MTP Uid", typeof(string), typeof(TestCase)); @@ -31,53 +62,53 @@ internal static class MtpTestNodeConverter /// Returns true when the node represents a runnable test (a leaf "action" node) rather than a /// grouping node (namespace/class/suite). /// - public static bool IsActionNode(JsonObject node) - => MtpJson.GetString(node, MtpConstants.NodeType) is "action"; - - public static string? GetExecutionState(JsonObject node) - => MtpJson.GetString(node, MtpConstants.ExecutionState); + public static bool IsActionNode(MtpTestNodeUpdate update) + => update.NodeType is ActionNodeType; - public static TestCase ToTestCase(JsonObject node, string source) + public static TestCase ToTestCase(MtpTestNodeUpdate update, string source) { - string uid = MtpJson.GetString(node, MtpConstants.Uid) ?? Guid.NewGuid().ToString(); - string fullyQualifiedName = MtpJson.GetString(node, MtpConstants.VsTestFullyQualifiedName) ?? uid; - string executorUri = MtpJson.GetString(node, MtpConstants.VsTestExecutorUri) ?? MtpConstants.DefaultExecutorUri; + string? uid = update.Uid; + string fullyQualifiedName = GetRawString(update, VsTestFullyQualifiedNameKey) + ?? (uid is { Length: > 0 } ? uid : Guid.NewGuid().ToString()); + string executorUri = GetRawString(update, VsTestExecutorUriKey) ?? DefaultExecutorUri; var testCase = new TestCase(fullyQualifiedName, new Uri(executorUri), source) { - DisplayName = MtpJson.GetString(node, MtpConstants.DisplayName) ?? fullyQualifiedName, + DisplayName = update.DisplayName ?? fullyQualifiedName, }; - testCase.SetPropertyValue(MtpUidProperty, uid); + if (uid is { Length: > 0 }) + { + testCase.SetPropertyValue(MtpUidProperty, uid); + } - string? file = MtpJson.GetString(node, MtpConstants.LocationFile); + string? file = GetRawString(update, LocationFileKey); if (!string.IsNullOrEmpty(file)) { testCase.CodeFilePath = file; - if (MtpJson.TryGetInt(node, MtpConstants.LocationLineStart, out int line)) + if (TryGetRawInt(update, LocationLineStartKey, out int line)) { testCase.LineNumber = line; } } - AddTraits(node, testCase); + AddTraits(update, testCase); return testCase; } - public static TestResult ToTestResult(JsonObject node, string source) + public static TestResult ToTestResult(MtpTestNodeUpdate update, string source) { - var testCase = ToTestCase(node, source); - string? state = GetExecutionState(node); + var testCase = ToTestCase(update, source); var result = new TestResult(testCase) { - Outcome = ToOutcome(state), + Outcome = ToOutcome(update.ExecutionState), DisplayName = testCase.DisplayName, - ErrorMessage = MtpJson.GetString(node, MtpConstants.ErrorMessage), - ErrorStackTrace = MtpJson.GetString(node, MtpConstants.ErrorStackTrace), + ErrorMessage = update.ErrorMessage, + ErrorStackTrace = update.ErrorStackTrace, }; - if (MtpJson.TryGetDouble(node, MtpConstants.TimeDurationMs, out double durationMs)) + if (update.DurationInMilliseconds is { } durationMs) { result.Duration = TimeSpan.FromMilliseconds(durationMs); } @@ -85,13 +116,13 @@ public static TestResult ToTestResult(JsonObject node, string source) // Surface the test's captured standard output/error (when the MTP node carries it) as result // messages so the console and TRX loggers show it, matching the classic path where a test's // stdout/stderr is attached to its result. - string? standardOutput = MtpJson.GetString(node, MtpConstants.StandardOutput); + string? standardOutput = GetRawString(update, StandardOutputKey); if (!string.IsNullOrEmpty(standardOutput)) { result.Messages.Add(new TestResultMessage(TestResultMessage.StandardOutCategory, standardOutput)); } - string? standardError = MtpJson.GetString(node, MtpConstants.StandardError); + string? standardError = GetRawString(update, StandardErrorKey); if (!string.IsNullOrEmpty(standardError)) { result.Messages.Add(new TestResultMessage(TestResultMessage.StandardErrorCategory, standardError)); @@ -101,42 +132,109 @@ public static TestResult ToTestResult(JsonObject node, string source) } public static bool IsTerminalState(string? state) - => state is MtpConstants.StatePassed - or MtpConstants.StateFailed - or MtpConstants.StateSkipped - or MtpConstants.StateError - or MtpConstants.StateTimedOut; + => state is StatePassed + or StateFailed + or StateSkipped + or StateError + or StateTimedOut; + + public static bool IsInProgressState(string? state) + => state is StateInProgress; private static TestOutcome ToOutcome(string? state) => state switch { - MtpConstants.StatePassed => TestOutcome.Passed, - MtpConstants.StateFailed => TestOutcome.Failed, - MtpConstants.StateError => TestOutcome.Failed, - MtpConstants.StateTimedOut => TestOutcome.Failed, - MtpConstants.StateSkipped => TestOutcome.Skipped, + StatePassed => TestOutcome.Passed, + StateFailed => TestOutcome.Failed, + StateError => TestOutcome.Failed, + StateTimedOut => TestOutcome.Failed, + StateSkipped => TestOutcome.Skipped, _ => TestOutcome.None, }; - private static void AddTraits(JsonObject node, TestCase testCase) + private static void AddTraits(MtpTestNodeUpdate update, TestCase testCase) { - if (MtpJson.GetValue(node, MtpConstants.Traits) is not JsonArray traits) + if (!update.Node.TryGetValue(TraitsKey, out object? traitsValue) || traitsValue is not IEnumerable traits) { return; } foreach (object? traitObject in traits) { - if (traitObject is not JsonObject trait) + if (traitObject is not IDictionary trait) { continue; } - foreach (KeyValuePair property in trait) + foreach (KeyValuePair property in trait) { - string value = property.Value as string ?? string.Empty; - testCase.Traits.Add(new Trait(property.Key, value)); + testCase.Traits.Add(new Trait(property.Key, FormatTraitValue(property.Value))); } } } + + /// + /// Renders a trait value as text. Traits are strings on the wire, but the two formatters box + /// JSON scalars differently (Jsonite and System.Text.Json can each yield int, long, double or + /// bool), so a non-string value here means the server sent a scalar rather than that the value + /// is absent. Formatting it invariantly preserves the data; treating it as an empty string + /// would silently drop it on one formatter and not the other. + /// + private static string FormatTraitValue(object? value) + => value switch + { + null => string.Empty, + string text => text, + IFormattable formattable => formattable.ToString(null, CultureInfo.InvariantCulture), + _ => value.ToString() ?? string.Empty, + }; + + private static string? GetRawString(MtpTestNodeUpdate update, string key) + => update.Node.TryGetValue(key, out object? value) ? value as string : null; + + /// + /// Coerces a raw node value to . The formatters box JSON numbers differently + /// (int, long or double for the same wire value), so the value must be coerced rather than + /// cast. Fractional and out-of-range values are rejected rather than truncated or wrapped: a + /// changed line number is a plausible-looking wrong answer, whereas returning false leaves the + /// caller's property at its default and is visibly "not set". + /// + private static bool TryGetRawInt(MtpTestNodeUpdate update, string key, out int result) + { + switch (update.Node.TryGetValue(key, out object? value) ? value : null) + { + case int i: + result = i; + return true; + + case long l when l is >= int.MinValue and <= int.MaxValue: + result = (int)l; + return true; + + case double d + when d is >= int.MinValue and <= int.MaxValue + && d == Math.Truncate(d): + result = (int)d; + return true; + + case float f + // (float)int.MaxValue rounds up to 2147483648f, so comparing a float against + // int.MaxValue directly lets that value through and the cast then saturates. Widen to + // double first so the bound is exact. + when (double)f is >= int.MinValue and <= int.MaxValue + && f == Math.Truncate(f): + result = (int)f; + return true; + + case decimal m + when m is >= int.MinValue and <= int.MaxValue + && m == decimal.Truncate(m): + result = (int)m; + return true; + + default: + result = 0; + return false; + } + } } diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Microsoft.TestPlatform.CrossPlatEngine.csproj b/src/Microsoft.TestPlatform.CrossPlatEngine/Microsoft.TestPlatform.CrossPlatEngine.csproj index 1242e70814..b094b9f61b 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Microsoft.TestPlatform.CrossPlatEngine.csproj +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Microsoft.TestPlatform.CrossPlatEngine.csproj @@ -5,6 +5,12 @@ Microsoft.TestPlatform.CrossPlatEngine $(NetFrameworkMinimum);$(ExtensionTargetFrameworks);$(NetCoreAppMinimum) false + + $(DefineConstants);MTP_CLIENT_EXCLUDE_NULLABLE_ATTRIBUTES @@ -21,6 +27,11 @@ true + + + + diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/Resources.Designer.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/Resources.Designer.cs index 5f3bfcb978..b66cc61000 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/Resources.Designer.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/Resources.Designer.cs @@ -331,6 +331,15 @@ internal static string ProxyIsAlreadyAvailable { } } + /// + /// Looks up a localized string similar to Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection.. + /// + internal static string MtpTestCaseMissingNodeUid { + get { + return ResourceManager.GetString("MtpTestCaseMissingNodeUid", resourceCulture); + } + } + /// /// Looks up a localized string similar to Skipping source:. /// diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/Resources.resx b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/Resources.resx index 15d57e9d62..3716ecef32 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/Resources.resx +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/Resources.resx @@ -229,6 +229,9 @@ No suitable test runtime provider was found: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Skipping source: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.cs.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.cs.xlf index 6bfe73ae1c..27582f38e6 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.cs.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.cs.xlf @@ -182,6 +182,11 @@ Nenašel se žádný vhodný zprostředkovatel testovacího modulu runtime: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: Přeskočení zdroje: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.de.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.de.xlf index a1b4810b3a..d8a8450825 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.de.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.de.xlf @@ -182,6 +182,11 @@ Es wurde kein geeigneter Testruntimeanbieter gefunden: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: Quelle wird übersprungen: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.es.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.es.xlf index 4f88016d94..2e64425856 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.es.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.es.xlf @@ -182,6 +182,11 @@ No se encontró ningún proveedor de tiempo de ejecución de prueba adecuado: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: Omitiendo origen: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.fr.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.fr.xlf index 2e6e8fd03d..991f696b71 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.fr.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.fr.xlf @@ -182,6 +182,11 @@ Aucun fournisseur de runtime de test approprié n’a été trouvé : + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: Source ignorée : diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.it.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.it.xlf index c6c78eb407..ef2732073f 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.it.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.it.xlf @@ -182,6 +182,11 @@ Non è stato trovato alcun provider di runtime di test appropriato: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: L'origine verrà ignorata: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ja.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ja.xlf index aba3c1288b..ca2d25d0c9 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ja.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ja.xlf @@ -182,6 +182,11 @@ 適切なテスト ランタイム プロバイダーが見つかりませんでした。 + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: ソースをスキップしています: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ko.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ko.xlf index e8e5edff47..1ac9b1f559 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ko.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ko.xlf @@ -182,6 +182,11 @@ 적합한 테스트 런타임 공급자를 찾을 수 없습니다. + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: 원본을 건너뛰는 중: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.pl.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.pl.xlf index 7254f8c97e..6cc5812677 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.pl.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.pl.xlf @@ -182,6 +182,11 @@ Nie znaleziono odpowiedniego dostawcy środowiska uruchomieniowego testu: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: Pomijanie źródła: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.pt-BR.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.pt-BR.xlf index 248a5a7248..37911f9522 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.pt-BR.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.pt-BR.xlf @@ -182,6 +182,11 @@ Nenhum provedor de runtime de teste encontrado: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: Ignorando origem: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ru.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ru.xlf index 3392873c33..7161d35466 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ru.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.ru.xlf @@ -182,6 +182,11 @@ Не найден подходящий поставщик среды выполнения теста: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: Пропуск источника: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.tr.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.tr.xlf index 4f05cc5c29..97ebc4138f 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.tr.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.tr.xlf @@ -182,6 +182,11 @@ Uygun bir test çalışma zamanı sağlayıcısı bulunamadı: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: Kaynak atlanıyor: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.zh-Hans.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.zh-Hans.xlf index ad3cd5c151..26f3af22cf 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.zh-Hans.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.zh-Hans.xlf @@ -182,6 +182,11 @@ 找不到合适的测试运行时提供程序: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: 正在跳过源: diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.zh-Hant.xlf b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.zh-Hant.xlf index bbc67131a0..1ab3b9af8d 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.zh-Hant.xlf +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Resources/xlf/Resources.zh-Hant.xlf @@ -182,6 +182,11 @@ 找不到適合的測試執行階段提供者: + + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + Cannot run test '{0}' because it does not carry a Microsoft.Testing.Platform node identifier. The test case was not produced by a Microsoft.Testing.Platform discovery, or the identifier was lost in transit. Re-run discovery for this project, or run without a test selection. + + Skipping source: 正在略過來源: diff --git a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/MtpUnderVstestTests.cs b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/MtpUnderVstestTests.cs index 41cca51985..ea238bb9c9 100644 --- a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/MtpUnderVstestTests.cs +++ b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/MtpUnderVstestTests.cs @@ -23,10 +23,16 @@ public class MtpUnderVstestTests : AcceptanceTestBase { private const string MtpTestHostDisableFeatureFlag = "VSTEST_DISABLE_MTP_TESTHOST"; - // MtpMSTestProject is an MSTest project built as an MTP application (EnableMSTestRunner): three tests + // MtpMSTestProject is an MSTest project built as an MTP application (EnableMSTestRunner): four tests // pass, one fails, one is skipped. private const string MtpApp = "MtpMSTestProject.dll"; + // The display name of MtpMSTestProject's deliberately non-ASCII test. Its UTF-8 byte count exceeds + // its character count (German umlauts 2 bytes, Japanese 3 bytes, Czech caron 2 bytes), which is + // what exercises the MTP frame's byte-denominated Content-Length. Must match the DisplayName on + // MtpMSTestProject.UnitTests.TestNonAsciiDisplayName. + private const string NonAsciiTestName = "TestGrüße日本語Čau"; + // MSTestProject1 is a classic vstest MSTest project driven by the vstest testhost: one passes, one // fails, one is skipped. private const string ClassicApp = "MSTestProject1.dll"; @@ -74,7 +80,49 @@ public void RunMtpApplicationExecutesTestsOverMtpProtocol(RunnerInfo runnerInfo) InvokeVsTestWithMtpTestHostEnabled(arguments); - ValidateSummaryStatus(3, 1, 1); + ValidateSummaryStatus(4, 1, 1); + } + + [TestMethod] + // MTP frames declare Content-Length in UTF-8 bytes. A client that consumes that number of + // characters instead under-reads any frame carrying multi-byte content and desynchronizes the + // connection from the next message onward. Test names are user-authored and travel + // server-to-client on every node update, which makes them the realistic trigger. + // + // This is a name-integrity guard rather than a full reproduction of that framing bug: the .NET + // MTP server serializes with System.Text.Json, whose default encoder escapes non-ASCII to + // \uXXXX, so the bytes on the wire are ASCII and byte count coincidentally equals character + // count. The framing bug itself is proved at the unit level in testfx against the transport + // directly. What this guards is that a multi-byte name survives escaping, transport and decoding + // unchanged - which is what regressed in every historical variant of this bug. + [TestMatrix(testHost: Target.Net)] + public void RunMtpApplicationPreservesNonAsciiTestNames(RunnerInfo runnerInfo) + { + SetTestEnvironment(_testEnvironment, runnerInfo); + + var trxFileName = "nonascii.trx"; + var arguments = PrepareArguments( + GetAssetFullPath(MtpApp), + testAdapterPath: null, + runSettings: string.Empty, + FrameworkArgValue, + runnerInfo.InIsolationValue, + resultsDirectory: TempDirectory.Path); + arguments = string.Concat(arguments, $" /logger:trx;LogFileName={trxFileName}"); + + InvokeVsTestWithMtpTestHostEnabled(arguments); + + // The run completing at all is the primary assertion: a desynchronized frame corrupts the + // messages that follow, so the counts would not add up. + ValidateSummaryStatus(4, 1, 1); + + var trxPath = Path.Combine(TempDirectory.Path, trxFileName); + Assert.IsTrue(File.Exists(trxPath), "Expected a TRX at '{0}'.", trxPath); + var trx = File.ReadAllText(trxPath); + Assert.Contains( + NonAsciiTestName, + trx, + "Expected the multi-byte UTF-8 test name to survive the MTP transport intact."); } [TestMethod] @@ -97,7 +145,7 @@ public void RunMixedClassicAndMtpApplicationsInSingleRun(RunnerInfo runnerInfo) InvokeVsTestWithMtpTestHostEnabled(arguments); // Classic 1/1/1 + MTP 3/1/1 aggregated into one run summary. - ValidateSummaryStatus(4, 2, 2); + ValidateSummaryStatus(5, 2, 2); } [TestMethod] @@ -120,7 +168,7 @@ public void RunMixedClassicAndMtpApplicationsWritesSingleTrx(RunnerInfo runnerIn InvokeVsTestWithMtpTestHostEnabled(arguments); - ValidateSummaryStatus(4, 2, 2); + ValidateSummaryStatus(5, 2, 2); var trxPath = Path.Combine(TempDirectory.Path, trxFileName); Assert.IsTrue(File.Exists(trxPath), "Expected a single TRX to be written for the mixed run at '{0}'.", trxPath); @@ -149,7 +197,7 @@ public void RunMtpApplicationWithBlameCompletesRun(RunnerInfo runnerInfo) InvokeVsTestWithMtpTestHostEnabled(arguments); - ValidateSummaryStatus(3, 1, 1); + ValidateSummaryStatus(4, 1, 1); } [TestMethod] @@ -190,7 +238,7 @@ public void RunMtpApplicationInjectsRunSettingsEnvironmentVariables(RunnerInfo r InvokeVsTestWithMtpTestHostEnabled(arguments, env); // The guarded test passes only if MTP_FROM_RUNSETTINGS reached the host with the runsettings value. - ValidateSummaryStatus(3, 1, 1); + ValidateSummaryStatus(4, 1, 1); } [TestMethod] @@ -215,8 +263,8 @@ public void RunMtpApplicationSurfacesPerTestStandardOutput(RunnerInfo runnerInfo InvokeVsTestWithMtpTestHostEnabled(arguments); - // MtpMSTestProject has five test cases: three pass, one fails, one is skipped. - ValidateSummaryStatus(3, 1, 1); + // MtpMSTestProject has six test cases: four pass, one fails, one is skipped. + ValidateSummaryStatus(4, 1, 1); var trxPath = Path.Combine(TempDirectory.Path, trxFileName); Assert.IsTrue(File.Exists(trxPath), "Expected a TRX at '{0}'.", trxPath); @@ -263,8 +311,8 @@ public void RunMtpApplicationWithGenericOutOfProcDataCollectorCompletesRun(Runne InvokeVsTestWithMtpTestHostEnabled(arguments, env); // The run must complete with the usual summary rather than hang at shutdown. MtpMSTestProject has - // five test cases: three pass, one fails, one is skipped. - ValidateSummaryStatus(3, 1, 1); + // six test cases: four pass, one fails, one is skipped. + ValidateSummaryStatus(4, 1, 1); // The datacollector lifecycle must be driven end to end even though there is no testhost: the // session events, the launched-process notification and the forwarded per-test-case events all @@ -277,15 +325,15 @@ public void RunMtpApplicationWithGenericOutOfProcDataCollectorCompletesRun(Runne StdOutputContains("Data collector 'SampleDataCollector' message: TestCaseEnded"); // The collector emits one attachment per started test case through the forwarded TestCaseStart - // events. All five MtpMSTestProject test cases surface a start on this path (the skipped one still - // reports a TestCaseStart), so five attachments must land in the results directory. Exclude the + // events. All six MtpMSTestProject test cases surface a start on this path (the skipped one still + // reports a TestCaseStart), so six attachments must land in the results directory. Exclude the // collector's own source directory so only the moved attachments are counted. var collectorSourceDirectoryPrefix = collectorSourceDirectory + Path.DirectorySeparatorChar; var testCaseAttachments = Directory .GetFiles(TempDirectory.Path, "testcasefilename*.txt", SearchOption.AllDirectories) .Where(file => !file.StartsWith(collectorSourceDirectoryPrefix, StringComparison.OrdinalIgnoreCase)) .ToList(); - Assert.HasCount(5, testCaseAttachments, "Expected one per-test-case attachment for each started MtpMSTestProject test case forwarded on the MTP path."); + Assert.HasCount(6, testCaseAttachments, "Expected one per-test-case attachment for each started MtpMSTestProject test case forwarded on the MTP path."); } private void InvokeVsTestWithMtpTestHostEnabled(string arguments, Dictionary? environmentVariables = null) diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/FakeMtpServerClient.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/FakeMtpServerClient.cs new file mode 100644 index 0000000000..0dd8bb0570 --- /dev/null +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/FakeMtpServerClient.cs @@ -0,0 +1,137 @@ +// 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.Threading; +using System.Threading.Tasks; + +using Microsoft.Testing.Platform.ServerMode.Client; + +namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.UnitTests.Client.MTP; + +/// +/// An in-memory that records how the proxy managers drive it, so the +/// launch, discover, run and shutdown sequence can be asserted without starting a real MTP +/// application. +/// +internal sealed class FakeMtpServerClient : IMtpServerClient +{ + public event EventHandler? TestNodesUpdated; + + public event EventHandler? LogReceived; + +#pragma warning disable CS0067 // Required by IMtpServerClient; the proxy managers do not subscribe to these. + public event EventHandler? TelemetryReceived; + + public event EventHandler? AttachmentsReceived; +#pragma warning restore CS0067 + + public int ProcessId { get; set; } = 4242; + + public MtpServerCapabilities? Capabilities { get; private set; } + + public Func?, CancellationToken, Task?>>? ServerRequestHandler { get; set; } + + /// Gets a value indicating whether was called. + public bool ExitCalled { get; private set; } + + /// Gets the cancellation token was called with. + public CancellationToken ExitToken { get; private set; } + + public bool Disposed { get; private set; } + + /// Gets the uids the manager asked the server to run, when a filtered run was requested. + public IReadOnlyCollection? RunFilterUids { get; private set; } + + /// Gets or sets the nodes the fake server pushes while handling discover or run. + public IReadOnlyList NodesToPush { get; set; } = []; + + /// Gets or sets an exception the fake server throws from discover or run. + public Exception? ThrowFromRequest { get; set; } + + /// Gets or sets a delay applied to , simulating a wedged server. + public TimeSpan ExitDelay { get; set; } = TimeSpan.Zero; + + /// Gets or sets an exception throws, simulating a refused shutdown. + public Exception? ThrowFromExit { get; set; } + + public Task InitializeAsync(CancellationToken cancellationToken = default) + { + Capabilities = new MtpServerCapabilities( + serverProcessId: ProcessId, + serverName: "FakeMtpServer", + serverVersion: "1.0.0", + supportsDiscovery: true, + multiRequestSupport: false, + vstestProviderSupport: false, + supportsAttachments: true, + multiConnectionProvider: false); + return Task.FromResult(Capabilities); + } + + public Task DiscoverTestsAsync(CancellationToken cancellationToken = default) + { + PushNodes(); + return ThrowFromRequest is not null ? Task.FromException(ThrowFromRequest) : Task.CompletedTask; + } + + public Task DiscoverTestsAsync(IReadOnlyCollection testNodeUids, CancellationToken cancellationToken = default) + => DiscoverTestsAsync(cancellationToken); + + public Task DiscoverTestsWithFilterAsync(string graphFilter, CancellationToken cancellationToken = default) + => DiscoverTestsAsync(cancellationToken); + + public Task RunTestsAsync(CancellationToken cancellationToken = default) + => CompleteRun(); + + public Task RunTestsAsync(IReadOnlyCollection testNodeUids, CancellationToken cancellationToken = default) + { + RunFilterUids = testNodeUids; + return CompleteRun(); + } + + public Task RunTestsWithFilterAsync(string graphFilter, CancellationToken cancellationToken = default) + => CompleteRun(); + + public async Task ExitAsync(CancellationToken cancellationToken = default) + { + ExitCalled = true; + ExitToken = cancellationToken; + + if (ThrowFromExit is not null) + { + throw ThrowFromExit; + } + + // Honour the token so a caller that passes an already-cancelled token observes the throw a + // real client would produce. + cancellationToken.ThrowIfCancellationRequested(); + + if (ExitDelay > TimeSpan.Zero) + { + await Task.Delay(ExitDelay, cancellationToken).ConfigureAwait(false); + } + } + + public void Dispose() => Disposed = true; + + public void RaiseLog(string level, string message) + => LogReceived?.Invoke(this, new MtpLogEventArgs(level, message)); + + private void PushNodes() + { + if (NodesToPush.Count > 0) + { + TestNodesUpdated?.Invoke(this, new MtpTestNodeUpdateEventArgs(Guid.NewGuid(), NodesToPush)); + } + } + + private Task CompleteRun() + { + PushNodes(); + return ThrowFromRequest is not null + ? Task.FromException(ThrowFromRequest) + : Task.FromResult(new MtpRunResult([])); + } +} diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpClientOptionsFactoryTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpClientOptionsFactoryTests.cs new file mode 100644 index 0000000000..19642e33c9 --- /dev/null +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpClientOptionsFactoryTests.cs @@ -0,0 +1,113 @@ +// 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.Testing.Platform.ServerMode.Client; +using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Helpers; +using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.UnitTests.Client.MTP; + +/// +/// Not parallelized: these tests mutate the process-wide VSTEST_CONNECTION_TIMEOUT environment +/// variable, which any concurrently running test that reads +/// would observe. +/// +[TestClass] +[DoNotParallelize] +public class MtpClientOptionsFactoryTests +{ + private string? _originalTimeout; + + [TestInitialize] + public void Initialize() + => _originalTimeout = Environment.GetEnvironmentVariable(EnvironmentHelper.VstestConnectionTimeout); + + [TestCleanup] + public void Cleanup() + => Environment.SetEnvironmentVariable(EnvironmentHelper.VstestConnectionTimeout, _originalTimeout); + + [TestMethod] + public void CreateOptionsIdentifiesVstestAsAStatelessClient() + { + MtpServerClientOptions options = MtpClientOptionsFactory.CreateOptions(); + + Assert.AreEqual("vstest", options.ClientName); + Assert.IsFalse(options.IsStateful, "vstest drives a single discover-or-run session per launch."); + Assert.IsFalse(options.DebuggerProvider); + Assert.IsNotNull(options.Logger); + } + + /// + /// The MTP connection timeout must follow vstest's shared VSTEST_CONNECTION_TIMEOUT knob so a + /// user extending the timeout for a slow environment affects the MTP path exactly as it affects + /// every other vstest connection. + /// + [TestMethod] + public void CreateOptionsHonoursTheSharedConnectionTimeoutOverride() + { + Environment.SetEnvironmentVariable(EnvironmentHelper.VstestConnectionTimeout, "300"); + + MtpServerClientOptions options = MtpClientOptionsFactory.CreateOptions(); + + Assert.AreEqual(TimeSpan.FromSeconds(300), options.ConnectionTimeout); + } + + [TestMethod] + public void CreateOptionsFallsBackToTheSharedDefaultConnectionTimeout() + { + Environment.SetEnvironmentVariable(EnvironmentHelper.VstestConnectionTimeout, null); + + MtpServerClientOptions options = MtpClientOptionsFactory.CreateOptions(); + + Assert.AreEqual(TimeSpan.FromSeconds(EnvironmentHelper.DefaultConnectionTimeout), options.ConnectionTimeout); + } + + [TestMethod] + public void CreateOptionsIgnoresAnUnparsableConnectionTimeout() + { + Environment.SetEnvironmentVariable(EnvironmentHelper.VstestConnectionTimeout, "not-a-number"); + + MtpServerClientOptions options = MtpClientOptionsFactory.CreateOptions(); + + Assert.AreEqual(TimeSpan.FromSeconds(EnvironmentHelper.DefaultConnectionTimeout), options.ConnectionTimeout); + } + + [TestMethod] + public void CreateOptionsCopiesEnvironmentVariables() + { + var variables = new Dictionary + { + ["FOO"] = "bar", + ["EMPTY"] = null, + }; + + MtpServerClientOptions options = MtpClientOptionsFactory.CreateOptions(variables); + + Assert.AreEqual("bar", options.EnvironmentVariables["FOO"]); + Assert.IsNull(options.EnvironmentVariables["EMPTY"]); + } + + [TestMethod] + public void CreateOptionsAcceptsNoEnvironmentVariables() + { + MtpServerClientOptions options = MtpClientOptionsFactory.CreateOptions(null); + + Assert.IsEmpty(options.EnvironmentVariables); + } + + [TestMethod] + [DataRow("Error", TestMessageLevel.Error)] + [DataRow("Critical", TestMessageLevel.Error)] + [DataRow("Warning", TestMessageLevel.Warning)] + [DataRow("Information", TestMessageLevel.Informational)] + [DataRow("Debug", TestMessageLevel.Informational)] + [DataRow("Trace", TestMessageLevel.Informational)] + [DataRow("a-level-the-server-added-later", TestMessageLevel.Informational)] + public void MapServerLogLevelMapsOntoVstestMessageLevels(string level, TestMessageLevel expected) + => Assert.AreEqual(expected, MtpClientOptionsFactory.MapServerLogLevel(level)); +} diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpProxyDiscoveryManagerTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpProxyDiscoveryManagerTests.cs new file mode 100644 index 0000000000..80e92b5703 --- /dev/null +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpProxyDiscoveryManagerTests.cs @@ -0,0 +1,170 @@ +// 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.Testing.Platform.ServerMode.Client; +using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; +using Microsoft.VisualStudio.TestPlatform.ObjectModel; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Moq; + +namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.UnitTests.Client.MTP; + +/// +/// Tests that drive end to end against a fake MTP server, +/// so the launch/initialize/discover/exit sequence is asserted without starting a real test +/// application. +/// +/// +/// Not parallelized: these tests swap the process-wide +/// seam, so running them alongside another class that does the same would let one class's fake leak +/// into the other's run. +/// +[TestClass] +[DoNotParallelize] +public class MtpProxyDiscoveryManagerTests +{ + private const string Source = @"C:\tests\MtpApp.dll"; + + private Func? _originalLaunch; + private FakeMtpServerClient _client = null!; + private Mock _eventHandler = null!; + + [TestInitialize] + public void Initialize() + { + _originalLaunch = MtpServerClientFactory.Launch; + _client = new FakeMtpServerClient(); + MtpServerClientFactory.Launch = (_, _) => _client; + _eventHandler = new Mock(); + } + + [TestCleanup] + public void Cleanup() + => MtpServerClientFactory.Launch = _originalLaunch!; + + private static DiscoveryCriteria Criteria() + => new([Source], 1, ""); + + private static MtpTestNodeUpdate ActionNode(string uid, string displayName) + => new( + new Dictionary + { + ["uid"] = uid, + ["display-name"] = displayName, + ["node-type"] = "action", + }, + parentUid: null); + + private static MtpTestNodeUpdate ActionNodeWithoutUid(string displayName) + => new( + new Dictionary + { + ["display-name"] = displayName, + ["node-type"] = "action", + }, + parentUid: null); + + [TestMethod] + public void DiscoverTestsReportsDiscoveredActionNodes() + { + _client.NodesToPush = [ActionNode("uid-1", "TestOne"), ActionNode("uid-2", "TestTwo")]; + + List? discovered = null; + _eventHandler + .Setup(h => h.HandleDiscoveredTests(It.IsAny>())) + .Callback>(tests => discovered = [.. tests]); + + using var manager = new MtpProxyDiscoveryManager(); + manager.DiscoverTests(Criteria(), _eventHandler.Object); + + Assert.IsNotNull(discovered); + Assert.HasCount(2, discovered); + _eventHandler.Verify( + h => h.HandleDiscoveryComplete(It.Is(e => e.TotalCount == 2 && !e.IsAborted), null), + Times.Once); + } + + [TestMethod] + public void DiscoverTestsAsksTheServerToExit() + { + using var manager = new MtpProxyDiscoveryManager(); + manager.DiscoverTests(Criteria(), _eventHandler.Object); + + Assert.IsTrue(_client.ExitCalled); + Assert.IsTrue(_client.Disposed); + } + + [TestMethod] + public void SelectedRunFailsWhenDiscoveredActionNodeHasNoUid() + { + _client.NodesToPush = [ActionNodeWithoutUid("MissingUid")]; + + List? discovered = null; + _eventHandler + .Setup(h => h.HandleDiscoveredTests(It.IsAny>())) + .Callback>(tests => discovered = [.. tests]); + + using (var discoveryManager = new MtpProxyDiscoveryManager()) + { + discoveryManager.DiscoverTests(Criteria(), _eventHandler.Object); + } + + Assert.IsNotNull(discovered); + Assert.HasCount(1, discovered); + + var runClient = new FakeMtpServerClient(); + MtpServerClientFactory.Launch = (_, _) => runClient; + var runEventHandler = new Mock(); + + using var executionManager = new MtpProxyExecutionManager(); + executionManager.StartTestRun(new TestRunCriteria(discovered, 1), runEventHandler.Object); + + Assert.IsNull(runClient.RunFilterUids, "No run may be requested for a node the server did not identify."); + runEventHandler.Verify( + h => h.HandleLogMessage(TestMessageLevel.Error, It.IsAny()), + Times.AtLeastOnce); + } + + /// + /// Cancelling a run cancels the token the in-flight request is riding on. Before this fix the + /// manager awaited exit on that same token, so the graceful shutdown was skipped in exactly the + /// case it matters most. Exit now runs on its own bounded token from a finally block. + /// + [TestMethod] + public void DiscoverTestsStillExitsWhenDiscoveryIsCancelled() + { + _client.ThrowFromRequest = new OperationCanceledException(); + + using var manager = new MtpProxyDiscoveryManager(); + manager.DiscoverTests(Criteria(), _eventHandler.Object); + + Assert.IsTrue(_client.ExitCalled, "A cancelled discovery must still shut the test application down."); + Assert.IsFalse( + _client.ExitToken.IsCancellationRequested, + "Exit must not be driven by the cancelled run token, or it would be skipped."); + Assert.IsTrue(_client.Disposed); + } + + /// + /// A failure part-way through discovery must not leak the launched test application. + /// + [TestMethod] + public void DiscoverTestsExitsWhenDiscoveryFails() + { + _client.ThrowFromRequest = new InvalidOperationException("server blew up"); + + using var manager = new MtpProxyDiscoveryManager(); + manager.DiscoverTests(Criteria(), _eventHandler.Object); + + Assert.IsTrue(_client.ExitCalled, "Exit runs in a finally block, so a failed discovery still shuts down."); + Assert.IsTrue(_client.Disposed); + _eventHandler.Verify(h => h.HandleLogMessage(TestMessageLevel.Error, It.IsAny()), Times.Once); + } +} diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpProxyExecutionManagerTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpProxyExecutionManagerTests.cs new file mode 100644 index 0000000000..c13f8e5528 --- /dev/null +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpProxyExecutionManagerTests.cs @@ -0,0 +1,167 @@ +// 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.Linq; + +using Microsoft.Testing.Platform.ServerMode.Client; +using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; +using Microsoft.VisualStudio.TestPlatform.ObjectModel; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client; +using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Moq; + +namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.UnitTests.Client.MTP; + +/// +/// Tests that drive against a fake MTP server. +/// +/// +/// Not parallelized: these tests swap the process-wide +/// seam, so running them alongside another class that does the same would let one class's fake leak +/// into the other's run. +/// +[TestClass] +[DoNotParallelize] +public class MtpProxyExecutionManagerTests +{ + private const string Source = @"C:\tests\MtpApp.dll"; + + private Func? _originalLaunch; + private FakeMtpServerClient _client = null!; + private Mock _eventHandler = null!; + + [TestInitialize] + public void Initialize() + { + _originalLaunch = MtpServerClientFactory.Launch; + _client = new FakeMtpServerClient(); + MtpServerClientFactory.Launch = (_, _) => _client; + _eventHandler = new Mock(); + } + + [TestCleanup] + public void Cleanup() + => MtpServerClientFactory.Launch = _originalLaunch!; + + private static TestCase TestCaseWithUid(string uid) + { + var testCase = new TestCase("My.Tests.MyTest", new Uri(MtpTestNodeConverter.DefaultExecutorUri), Source); + testCase.SetPropertyValue(MtpTestNodeConverter.MtpUidProperty, uid); + return testCase; + } + + private static TestCase TestCaseWithoutUid() + => new("My.Tests.MyTest", new Uri(MtpTestNodeConverter.DefaultExecutorUri), Source); + + private static TestRunCriteria CriteriaFor(params TestCase[] tests) + => new(tests, 1); + + /// + /// The server matches a run filter on node uid alone, so the uid stored at discovery is what + /// must be sent - not the display name or the fully qualified name. + /// + [TestMethod] + public void StartTestRunSendsTheMtpNodeUidAsTheRunFilter() + { + using var manager = new MtpProxyExecutionManager(); + manager.StartTestRun(CriteriaFor(TestCaseWithUid("node-uid-1")), _eventHandler.Object); + + Assert.IsNotNull(_client.RunFilterUids); + Assert.AreEqual("node-uid-1", _client.RunFilterUids.Single()); + } + + /// + /// A TestCase with no MTP uid cannot be addressed: the server would match nothing and the run + /// would report success having executed zero of the selected tests. The manager must surface + /// that as an error instead of silently running nothing. + /// + [TestMethod] + public void StartTestRunFailsLoudlyWhenATestCarriesNoMtpUid() + { + using var manager = new MtpProxyExecutionManager(); + manager.StartTestRun(CriteriaFor(TestCaseWithoutUid()), _eventHandler.Object); + + Assert.IsNull(_client.RunFilterUids, "No run may be requested when the selection cannot be expressed."); + _eventHandler.Verify( + h => h.HandleLogMessage(ObjectModel.Logging.TestMessageLevel.Error, It.IsAny()), + Times.AtLeastOnce); + } + + /// + /// The whole source is aborted rather than silently running the addressable subset: reporting a + /// partial run as if it were the run the user asked for is the same class of bug this fix exists + /// to remove. + /// + [TestMethod] + public void StartTestRunFailsLoudlyWhenOnlySomeTestsCarryAnMtpUid() + { + using var manager = new MtpProxyExecutionManager(); + manager.StartTestRun(CriteriaFor(TestCaseWithUid("node-uid-1"), TestCaseWithoutUid()), _eventHandler.Object); + + Assert.IsNull( + _client.RunFilterUids, + "A selection that cannot be fully expressed must not be partially run."); + _eventHandler.Verify( + h => h.HandleLogMessage(ObjectModel.Logging.TestMessageLevel.Error, It.IsAny()), + Times.AtLeastOnce); + } + + [TestMethod] + public void StartTestRunRunsEveryTestWhenNoSpecificTestsAreSelected() + { + using var manager = new MtpProxyExecutionManager(); + manager.StartTestRun(new TestRunCriteria([Source], 1), _eventHandler.Object); + + Assert.IsNull(_client.RunFilterUids, "An unfiltered run must not send a uid filter at all."); + Assert.IsTrue(_client.ExitCalled); + } + + [TestMethod] + public void StartTestRunAsksTheServerToExitAndDisposesTheClient() + { + using var manager = new MtpProxyExecutionManager(); + manager.StartTestRun(CriteriaFor(TestCaseWithUid("node-uid-1")), _eventHandler.Object); + + Assert.IsTrue(_client.ExitCalled); + Assert.IsTrue(_client.Disposed); + } + + /// + /// Exit runs in a finally block, so a run that fails part-way through still shuts the test + /// application down rather than leaking the process. + /// + [TestMethod] + public void StartTestRunExitsWhenTheRunFails() + { + _client.ThrowFromRequest = new InvalidOperationException("server blew up"); + + using var manager = new MtpProxyExecutionManager(); + manager.StartTestRun(CriteriaFor(TestCaseWithUid("node-uid-1")), _eventHandler.Object); + + Assert.IsTrue(_client.ExitCalled); + Assert.IsTrue(_client.Disposed); + } + + /// + /// Cancelling a run cancels the token the in-flight request is riding on. Exit must not be tied + /// to that token, or a cancelled run would skip the shutdown handshake entirely. + /// + [TestMethod] + public void StartTestRunStillExitsWhenTheRunIsCancelled() + { + _client.ThrowFromRequest = new OperationCanceledException(); + + using var manager = new MtpProxyExecutionManager(); + manager.StartTestRun(CriteriaFor(TestCaseWithUid("node-uid-1")), _eventHandler.Object); + + Assert.IsTrue(_client.ExitCalled, "A cancelled run must still shut the test application down."); + Assert.IsFalse( + _client.ExitToken.IsCancellationRequested, + "Exit must not be driven by the cancelled run token, or it would be skipped."); + Assert.IsTrue(_client.Disposed, "The launched test application must never outlive a cancelled run."); + } +} diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpServerClientFactoryTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpServerClientFactoryTests.cs new file mode 100644 index 0000000000..b2dbaaa02c --- /dev/null +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpServerClientFactoryTests.cs @@ -0,0 +1,110 @@ +// 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.Diagnostics; +using System.Threading; + +using Microsoft.Testing.Platform.ServerMode.Client; +using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.UnitTests.Client.MTP; + +/// +/// Regression tests for the MTP client shutdown path. +/// +/// Before the retarget, exit was a fire-and-forget notification with no cancellation token. It is +/// now an awaited request/response call, which introduced two failure modes these tests pin: +/// passing the run's own (possibly already-cancelled) token would skip the handshake exactly when a +/// run is aborted, and an unbounded await would let a wedged test application hang the run. +/// +[TestClass] +public class MtpServerClientFactoryTests +{ + [TestMethod] + public void TryExitAsksTheServerToExit() + { + var client = new FakeMtpServerClient(); + + MtpServerClientFactory.TryExit(client); + + Assert.IsTrue(client.ExitCalled); + } + + /// + /// Cancelling or aborting a run is precisely when the run's token is already cancelled. Exit + /// must not be tied to it, or the graceful shutdown handshake would be skipped in the one case + /// it matters most. takes no token at all, so this + /// pins that it supplies an uncancelled, cancelable one of its own; the end-to-end proof that no + /// run token is plumbed through lives in the proxy-manager tests. + /// + [TestMethod] + public void TryExitSuppliesItsOwnUncancelledToken() + { + var client = new FakeMtpServerClient(); + + MtpServerClientFactory.TryExit(client); + + Assert.IsTrue(client.ExitCalled); + Assert.IsFalse( + client.ExitToken.IsCancellationRequested, + "Exit must run on a token that is not already cancelled."); + Assert.IsTrue( + client.ExitToken.CanBeCanceled, + "The token must be cancelable, otherwise the exit timeout could never fire."); + } + + /// + /// A test application that never acknowledges exit must not hang the run. Disposal (which the + /// caller performs afterwards) terminates the process regardless, so abandoning the handshake is + /// safe. + /// + [TestMethod] + public void TryExitGivesUpOnAnUnresponsiveServer() + { + var client = new FakeMtpServerClient { ExitDelay = TimeSpan.FromMinutes(5) }; + + var stopwatch = Stopwatch.StartNew(); + MtpServerClientFactory.TryExit(client); + stopwatch.Stop(); + + Assert.IsLessThan(TimeSpan.FromMinutes(1), stopwatch.Elapsed, "TryExit must be bounded by its own timeout."); + } + + [TestMethod] + public void TryExitSwallowsServerFailures() + { + var client = new FakeMtpServerClient { ThrowFromExit = new InvalidOperationException("server refused to exit") }; + + MtpServerClientFactory.TryExit(client); + + Assert.IsTrue(client.ExitCalled, "A failing exit must not propagate: the caller disposes the client next."); + } + + /// + /// The seam must default to the factory's own launcher, not to a test double. Asserting only + /// that it is non-null would pass for any delegate, including a fake another test class left + /// behind. Instead assert the delegate is implemented inside + /// itself - the default is a lambda, so its target lives in a compiler-generated closure nested + /// in the factory rather than on the factory type directly. + /// + [TestMethod] + public void LaunchDefaultsToTheRealClientLauncher() + { + Type? declaringType = MtpServerClientFactory.Launch.Method.DeclaringType; + + Assert.IsNotNull(declaringType); + Type outermost = declaringType; + while (outermost.DeclaringType is { } parent) + { + outermost = parent; + } + + Assert.AreEqual( + typeof(MtpServerClientFactory), + outermost, + "The default seam must be the factory's own launcher, not a test double left behind by another test."); + } +} diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpServerConnectionTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpServerConnectionTests.cs deleted file mode 100644 index d0f16c5c01..0000000000 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpServerConnectionTests.cs +++ /dev/null @@ -1,146 +0,0 @@ -// 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 Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; -using Microsoft.VisualStudio.TestTools.UnitTesting; - -namespace TestPlatform.CrossPlatEngine.UnitTests.Client.MTP; - -[TestClass] -public class MtpServerConnectionTests -{ - private const int Port = 12345; - - private string _tempDir = null!; - - [TestInitialize] - public void SetUp() - { - _tempDir = Path.Combine(Path.GetTempPath(), "mtp-buildlaunch-" + Guid.NewGuid().ToString("N")); - Directory.CreateDirectory(_tempDir); - } - - [TestCleanup] - public void TearDown() - { - try - { - if (Directory.Exists(_tempDir)) - { - Directory.Delete(_tempDir, recursive: true); - } - } - catch - { - // best-effort cleanup - } - } - - [TestMethod] - public void BuildLaunchWhenSourceIsExeLaunchesItDirectly() - { - string exe = Path.Combine(_tempDir, "Foo.exe"); - File.WriteAllText(exe, string.Empty); - - var (fileName, arguments, workingDirectory) = MtpServerConnection.BuildLaunch(exe, Port); - - Assert.AreEqual(exe, fileName); - Assert.DoesNotContain("\"", arguments); - Assert.AreEqual(_tempDir, workingDirectory); - } - - [TestMethod] - public void BuildLaunchWhenDllHasNoApphostFallsBackToDotnet() - { - string dll = Path.Combine(_tempDir, "Foo.dll"); - File.WriteAllText(dll, string.Empty); - - var (fileName, arguments, _) = MtpServerConnection.BuildLaunch(dll, Port); - - Assert.AreEqual("dotnet", fileName); - Assert.Contains($"\"{dll}\"", arguments); - } - - [TestMethod] - [OSCondition(OperatingSystems.Linux | OperatingSystems.OSX)] - public void BuildLaunchOnUnixIgnoresSiblingWindowsExeAndFallsBackToDotnet() - { - string dll = Path.Combine(_tempDir, "Foo.dll"); - File.WriteAllText(dll, string.Empty); - - // Stand in for a Windows PE apphost dragged along in a Windows-built payload that is then - // unzipped on Linux: the file exists but is not a native Unix executable. - File.WriteAllText(Path.Combine(_tempDir, "Foo.exe"), string.Empty); - - var (fileName, arguments, _) = MtpServerConnection.BuildLaunch(dll, Port); - - Assert.AreEqual("dotnet", fileName); - Assert.Contains($"\"{dll}\"", arguments); - } - - [TestMethod] - [OSCondition(OperatingSystems.Windows)] - public void BuildLaunchOnWindowsSelectsSiblingExeApphost() - { - string dll = Path.Combine(_tempDir, "Foo.dll"); - File.WriteAllText(dll, string.Empty); - - // On Windows the apphost is .exe and its mere presence is sufficient. - string exe = Path.Combine(_tempDir, "Foo.exe"); - File.WriteAllText(exe, string.Empty); - - var (fileName, arguments, _) = MtpServerConnection.BuildLaunch(dll, Port); - - Assert.AreEqual(exe, fileName); - Assert.DoesNotContain("\"", arguments); - } - - [TestMethod] - public void IsUsableApphostReturnsFalseWhenFileMissing() - { - Assert.IsFalse(MtpServerConnection.IsUsableApphost(Path.Combine(_tempDir, "does-not-exist"))); - } - -#if NET - [TestMethod] - [OSCondition(OperatingSystems.Linux | OperatingSystems.OSX)] - public void IsUsableApphostOnUnixReturnsFalseForNonExecutableFile() - { - string apphost = Path.Combine(_tempDir, "Foo"); - File.WriteAllText(apphost, string.Empty); - File.SetUnixFileMode(apphost, UnixFileMode.UserRead | UnixFileMode.UserWrite); - - Assert.IsFalse(MtpServerConnection.IsUsableApphost(apphost)); - } - - [TestMethod] - [OSCondition(OperatingSystems.Linux | OperatingSystems.OSX)] - public void IsUsableApphostOnUnixReturnsTrueForExecutableFile() - { - string apphost = Path.Combine(_tempDir, "Foo"); - File.WriteAllText(apphost, string.Empty); - File.SetUnixFileMode(apphost, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); - - Assert.IsTrue(MtpServerConnection.IsUsableApphost(apphost)); - } - - [TestMethod] - [OSCondition(OperatingSystems.Linux | OperatingSystems.OSX)] - public void BuildLaunchOnUnixSelectsExecutableExtensionlessApphost() - { - string dll = Path.Combine(_tempDir, "Foo.dll"); - File.WriteAllText(dll, string.Empty); - - string apphost = Path.Combine(_tempDir, "Foo"); - File.WriteAllText(apphost, string.Empty); - File.SetUnixFileMode(apphost, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); - - var (fileName, _, _) = MtpServerConnection.BuildLaunch(dll, Port); - - Assert.AreEqual(apphost, fileName); - } -#endif -} diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpTestNodeConverterTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpTestNodeConverterTests.cs new file mode 100644 index 0000000000..d8af911cee --- /dev/null +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/MTP/MtpTestNodeConverterTests.cs @@ -0,0 +1,358 @@ +// 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.Linq; + +using Microsoft.Testing.Platform.ServerMode.Client; +using Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.MTP; +using Microsoft.VisualStudio.TestPlatform.ObjectModel; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using TestResult = Microsoft.VisualStudio.TestPlatform.ObjectModel.TestResult; + +namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.UnitTests.Client.MTP; + +/// +/// Unit tests for . +/// +/// These pin the "normalized Node shape" contract the retarget onto +/// Microsoft.Testing.Platform.ServerMode.Client.Sources depends on. Both formatter paths (Jsonite on +/// net462/netstandard2.0, System.Text.Json on .NET) materialize every JSON object as a dictionary +/// and every array as a collection, but they box numbers differently (int/long/double). The +/// converter must therefore coerce numerics rather than hard-cast them, and these tests assert that +/// across every boxing the formatters can produce. +/// +[TestClass] +public class MtpTestNodeConverterTests +{ + private const string Source = @"C:\tests\MtpApp.dll"; + + private static MtpTestNodeUpdate Node(params (string Key, object? Value)[] properties) + { + var bag = new Dictionary + { + ["uid"] = "node-uid-1", + ["display-name"] = "MyTest", + ["node-type"] = "action", + }; + + foreach ((string key, object? value) in properties) + { + bag[key] = value; + } + + return new MtpTestNodeUpdate(bag, parentUid: null); + } + + private static MtpTestNodeUpdate RawNode(Dictionary bag) + => new(bag, parentUid: null); + + [TestMethod] + public void IsActionNodeReturnsTrueForActionNode() + => Assert.IsTrue(MtpTestNodeConverter.IsActionNode(Node(("node-type", "action")))); + + [TestMethod] + [DataRow("group")] + [DataRow("namespace")] + [DataRow("class")] + [DataRow("assembly")] + public void IsActionNodeReturnsFalseForGroupingNodes(string nodeType) + => Assert.IsFalse(MtpTestNodeConverter.IsActionNode(Node(("node-type", nodeType)))); + + [TestMethod] + public void IsActionNodeReturnsFalseWhenNodeTypeMissing() + => Assert.IsFalse(MtpTestNodeConverter.IsActionNode(RawNode(new Dictionary { ["uid"] = "u" }))); + + /// + /// The two formatters box JSON numbers differently: each can hand back int, long or double for + /// the same wire value. A hard cast throws - this is exactly the FormatException class of bug + /// that made the .NET axis fail every test before the package's number-decode fix - so the + /// converter coerces. Every boxing must produce the same line number. + /// + [TestMethod] + public void ToTestCaseCoercesLineNumberRegardlessOfNumericBoxing() + { + object[] boxings = [42, 42L, 42d, 42f, 42m]; + + foreach (object boxed in boxings) + { + TestCase testCase = MtpTestNodeConverter.ToTestCase( + Node(("location.file", @"C:\src\MyTest.cs"), ("location.line-start", boxed)), + Source); + + Assert.AreEqual(42, testCase.LineNumber, $"Boxing {boxed.GetType().Name} was not coerced."); + } + } + + [TestMethod] + public void ToTestCaseIgnoresLineNumberWhenLocationFileMissing() + { + TestCase testCase = MtpTestNodeConverter.ToTestCase(Node(("location.line-start", 42)), Source); + + Assert.IsNull(testCase.CodeFilePath); + } + + [TestMethod] + public void ToTestCaseIgnoresNonNumericLineNumber() + { + TestCase testCase = MtpTestNodeConverter.ToTestCase( + Node(("location.file", @"C:\src\MyTest.cs"), ("location.line-start", "not-a-number")), + Source); + + Assert.AreEqual(@"C:\src\MyTest.cs", testCase.CodeFilePath); + Assert.AreEqual(-1, testCase.LineNumber, "A non-numeric line-start must leave LineNumber at its TestCase default."); + } + + [TestMethod] + public void ToTestCaseUsesUidAsFullyQualifiedNameWhenBridgePropertiesAbsent() + { + TestCase testCase = MtpTestNodeConverter.ToTestCase(Node(), Source); + + Assert.AreEqual("node-uid-1", testCase.FullyQualifiedName); + Assert.AreEqual(MtpTestNodeConverter.DefaultExecutorUri, testCase.ExecutorUri.OriginalString); + Assert.AreEqual(Source, testCase.Source); + } + + [TestMethod] + public void ToTestCasePrefersBridgePropertiesWhenPresent() + { + TestCase testCase = MtpTestNodeConverter.ToTestCase( + Node( + ("vstest.TestCase.FullyQualifiedName", "My.Namespace.MyClass.MyTest"), + ("vstest.original-executor-uri", "executor://MSTestAdapter/v2")), + Source); + + Assert.AreEqual("My.Namespace.MyClass.MyTest", testCase.FullyQualifiedName); + Assert.AreEqual("executor://MSTestAdapter/v2", testCase.ExecutorUri.OriginalString); + } + + [TestMethod] + public void ToTestCaseFallsBackToFullyQualifiedNameWhenDisplayNameMissing() + { + TestCase testCase = MtpTestNodeConverter.ToTestCase( + RawNode(new Dictionary { ["uid"] = "only-uid", ["node-type"] = "action" }), + Source); + + Assert.AreEqual("only-uid", testCase.DisplayName); + } + + /// + /// The MTP node uid is the only identity the server matches a run filter on (it never reads the + /// display name), so the converter must stash it on the TestCase for the later filtered run. + /// + [TestMethod] + public void ToTestCaseStoresMtpUidProperty() + { + TestCase testCase = MtpTestNodeConverter.ToTestCase(Node(), Source); + + Assert.AreEqual("node-uid-1", testCase.GetPropertyValue(MtpTestNodeConverter.MtpUidProperty, null)); + } + + [TestMethod] + [DataRow("passed", TestOutcome.Passed)] + [DataRow("failed", TestOutcome.Failed)] + [DataRow("error", TestOutcome.Failed)] + [DataRow("timed-out", TestOutcome.Failed)] + [DataRow("skipped", TestOutcome.Skipped)] + [DataRow("in-progress", TestOutcome.None)] + [DataRow("discovered", TestOutcome.None)] + [DataRow("something-the-server-added-later", TestOutcome.None)] + public void ToTestResultMapsExecutionStateToOutcome(string state, TestOutcome expected) + { + TestResult result = MtpTestNodeConverter.ToTestResult(Node(("execution-state", state)), Source); + + Assert.AreEqual(expected, result.Outcome); + } + + [TestMethod] + public void ToTestResultMapsMissingExecutionStateToNone() + { + TestResult result = MtpTestNodeConverter.ToTestResult(Node(), Source); + + Assert.AreEqual(TestOutcome.None, result.Outcome); + } + + [TestMethod] + [DataRow("passed", true)] + [DataRow("failed", true)] + [DataRow("skipped", true)] + [DataRow("error", true)] + [DataRow("timed-out", true)] + [DataRow("in-progress", false)] + [DataRow("discovered", false)] + [DataRow(null, false)] + public void IsTerminalStateRecognizesTerminalStates(string? state, bool expected) + => Assert.AreEqual(expected, MtpTestNodeConverter.IsTerminalState(state)); + + [TestMethod] + [DataRow("in-progress", true)] + [DataRow("passed", false)] + [DataRow(null, false)] + public void IsInProgressStateRecognizesInProgress(string? state, bool expected) + => Assert.AreEqual(expected, MtpTestNodeConverter.IsInProgressState(state)); + + [TestMethod] + public void ToTestResultCarriesErrorMessageAndStackTrace() + { + TestResult result = MtpTestNodeConverter.ToTestResult( + Node( + ("execution-state", "failed"), + ("error.message", "Assert.AreEqual failed"), + ("error.stacktrace", " at MyTest()")), + Source); + + Assert.AreEqual("Assert.AreEqual failed", result.ErrorMessage); + Assert.AreEqual(" at MyTest()", result.ErrorStackTrace); + } + + [TestMethod] + public void ToTestResultMapsDurationWhenPresent() + { + TestResult result = MtpTestNodeConverter.ToTestResult( + Node(("execution-state", "passed"), ("time.duration-ms", 1234.5d)), + Source); + + Assert.AreEqual(TimeSpan.FromMilliseconds(1234.5), result.Duration); + } + + [TestMethod] + public void ToTestResultLeavesDurationUnsetWhenAbsent() + { + TestResult result = MtpTestNodeConverter.ToTestResult(Node(("execution-state", "passed")), Source); + + Assert.AreEqual(TimeSpan.Zero, result.Duration); + } + + [TestMethod] + public void ToTestResultAttachesStandardOutputAndError() + { + TestResult result = MtpTestNodeConverter.ToTestResult( + Node( + ("execution-state", "passed"), + ("standardOutput", "hello from the test"), + ("standardError", "a warning")), + Source); + + Assert.AreEqual( + "hello from the test", + result.Messages.Single(m => m.Category == TestResultMessage.StandardOutCategory).Text); + Assert.AreEqual( + "a warning", + result.Messages.Single(m => m.Category == TestResultMessage.StandardErrorCategory).Text); + } + + [TestMethod] + public void ToTestResultSkipsEmptyStandardStreams() + { + TestResult result = MtpTestNodeConverter.ToTestResult( + Node(("execution-state", "passed"), ("standardOutput", ""), ("standardError", null)), + Source); + + Assert.IsEmpty(result.Messages); + } + + [TestMethod] + public void ToTestCaseReadsStringTraits() + { + TestCase testCase = MtpTestNodeConverter.ToTestCase( + Node(("traits", new List { new Dictionary { ["Category"] = "Smoke" } })), + Source); + + Trait trait = testCase.Traits.Single(); + Assert.AreEqual("Category", trait.Name); + Assert.AreEqual("Smoke", trait.Value); + } + + /// + /// Traits are strings on the wire, but the formatters box JSON scalars differently. A non-string + /// trait value therefore means the server sent a scalar, not that the value is missing, so it + /// must be rendered rather than blanked - otherwise the same test loses trait data on one + /// formatter and keeps it on the other. + /// + [TestMethod] + public void ToTestCaseFormatsNonStringTraitValues() + { + TestCase testCase = MtpTestNodeConverter.ToTestCase( + Node(("traits", new List + { + new Dictionary + { + ["Priority"] = 1, + ["Timeout"] = 5000L, + ["Weight"] = 1.5d, + ["Enabled"] = true, + ["Missing"] = null, + }, + })), + Source); + + Dictionary traits = testCase.Traits.ToDictionary(t => t.Name, t => t.Value); + + Assert.AreEqual("1", traits["Priority"]); + Assert.AreEqual("5000", traits["Timeout"]); + Assert.AreEqual("1.5", traits["Weight"]); + Assert.AreEqual(bool.TrueString, traits["Enabled"]); + Assert.AreEqual(string.Empty, traits["Missing"]); + } + + /// + /// Numbers outside the Int32 range must be rejected rather than wrapped: a wrapped line number + /// is a plausible-looking wrong answer, whereas leaving the property at its default is visibly + /// "not set". + /// + [TestMethod] + public void ToTestCaseRejectsOutOfRangeLineNumberInsteadOfWrapping() + { + // The last entry is float: (float)int.MaxValue rounds *up* to 2147483648f, so a naive + // `f <= int.MaxValue` guard lets it through and the cast then saturates - the exact + // "plausible-looking wrong answer" the coercion exists to prevent. + object[] outOfRange = [(long)int.MaxValue + 1, (long)int.MinValue - 1, 1e18d, 2147483648f]; + + foreach (object boxed in outOfRange) + { + TestCase testCase = MtpTestNodeConverter.ToTestCase( + Node(("location.file", @"C:\src\MyTest.cs"), ("location.line-start", boxed)), + Source); + + Assert.AreEqual( + -1, + testCase.LineNumber, + $"Out-of-range value {boxed} ({boxed.GetType().Name}) must not be wrapped into a valid-looking line number."); + } + } + + [TestMethod] + public void ToTestCaseRejectsFractionalLineNumberInsteadOfTruncating() + { + object[] fractionalValues = [42.5d, -13.25f, 7.1m]; + + foreach (object boxed in fractionalValues) + { + TestCase testCase = MtpTestNodeConverter.ToTestCase( + Node(("location.file", @"C:\src\MyTest.cs"), ("location.line-start", boxed)), + Source); + + Assert.AreEqual( + -1, + testCase.LineNumber, + $"Fractional value {boxed} ({boxed.GetType().Name}) must not be truncated into a valid-looking line number."); + } + } + + [TestMethod] + public void ToTestCaseDoesNotThrowOnMalformedTraits() + { + TestCase notACollection = MtpTestNodeConverter.ToTestCase(Node(("traits", "nonsense")), Source); + Assert.AreEqual(0, notACollection.Traits.Count()); + + TestCase notDictionaries = MtpTestNodeConverter.ToTestCase( + Node(("traits", new List { "nonsense", 42 })), + Source); + Assert.AreEqual(0, notDictionaries.Traits.Count()); + + TestCase missing = MtpTestNodeConverter.ToTestCase(Node(), Source); + Assert.AreEqual(0, missing.Traits.Count()); + } +} diff --git a/test/TestAssets/MtpMSTestProject/UnitTests.cs b/test/TestAssets/MtpMSTestProject/UnitTests.cs index 2c625e7a01..6dfa280f05 100644 --- a/test/TestAssets/MtpMSTestProject/UnitTests.cs +++ b/test/TestAssets/MtpMSTestProject/UnitTests.cs @@ -41,6 +41,29 @@ public void TestSkipped() Assert.Fail("should never run"); } + // A test whose display name has a UTF-8 byte count greater than its character count: German + // umlauts (2 bytes each), Japanese (3 bytes each) and a Czech caron (2 bytes). + // + // MTP frames declare Content-Length in bytes, so a client that consumes that number of + // characters instead under-reads the frame and desynchronizes the connection from the next + // message onward. Test names are user-authored and travel server-to-client on every node update, + // which makes them the realistic trigger. + // + // Note this is a name-integrity guard, not a full reproduction of that framing bug: the .NET MTP + // server serializes with System.Text.Json, whose default encoder escapes non-ASCII to \uXXXX, so + // the bytes actually on the wire are ASCII and byte count happens to equal character count. The + // framing bug is proved at the unit level in testfx, against the transport directly. What this + // does guard is that the name survives escaping, transport and decoding unchanged. + // + // Deliberately no astral-plane character (emoji): those are escaped by System.Text.Json as a + // surrogate pair (\ud83c\udf89) and currently arrive in the TRX as that literal text rather than + // the character, which is a separate decoding defect being tracked on its own. + [TestMethod(DisplayName = "TestGrüße日本語Čau")] + public void TestNonAsciiDisplayName() + { + Assert.AreEqual(2, Add(1, 1)); + } + // Verifies that environment variables declared in a runsettings RunConfiguration/EnvironmentVariables // block are injected into the self-hosted MTP process. The check is opted into by the // CHECK_RUNSETTINGS_VAR control variable, which the env-var acceptance test passes as a *process* diff --git a/test/TestAssets/MtpPureProject/PureTestFramework.cs b/test/TestAssets/MtpPureProject/PureTestFramework.cs index 98a26fe60d..b4d982b667 100644 --- a/test/TestAssets/MtpPureProject/PureTestFramework.cs +++ b/test/TestAssets/MtpPureProject/PureTestFramework.cs @@ -17,15 +17,31 @@ namespace MtpPureProject; /// or Microsoft.TestPlatform.ObjectModel. It publishes test nodes over the MTP protocol directly, /// which is exactly what vstest's MTP provider consumes. /// -/// It exposes four tests, mirroring the MSTest asset so results are directly comparable: -/// - TestAddPasses : passes (exercises Calculator.Add) -/// - TestMultiplyPasses : passes (exercises Calculator.Multiply) -/// - TestFails : fails (throws) -/// - TestSkipped : skipped -/// Expected: Passed 2, Failed 1, Skipped 1, Total 4. +/// It exposes five tests, mirroring the MSTest asset so results are directly comparable: +/// - TestAddPasses : passes (exercises Calculator.Add) +/// - TestMultiplyPasses : passes (exercises Calculator.Multiply) +/// - TestFails : fails (throws) +/// - TestSkipped : skipped +/// - TestNonAsciiDisplayName: passes, and carries a multi-byte UTF-8 display name +/// Expected: Passed 3, Failed 1, Skipped 1, Total 5. /// internal sealed class PureTestFramework : ITestFramework, IDataProducer { + /// + /// A display name whose UTF-8 byte count exceeds its character count: German umlauts (2 bytes + /// each), Japanese (3 bytes each) and a Czech caron (2 bytes). + /// + /// The MTP frame header declares Content-Length in bytes, so a client that consumes that number + /// of characters instead under-reads the frame and desynchronizes the connection from the next + /// message onward. Test names are user-authored and flow server-to-client on every node update, + /// which makes this the realistic trigger. + /// + /// Note this asset is not currently referenced by any test in the repo - the acceptance coverage + /// lives on MtpMSTestProject, which every MTP scenario uses. This name is kept in step with that + /// asset so the two stay comparable if this one is ever wired up. + /// + internal const string NonAsciiTestName = "TestGrüße日本語Čau"; + private static readonly SessionUid SessionUid = new("PureMtpSession"); private static readonly TestDefinition[] Tests = @@ -47,6 +63,16 @@ internal sealed class PureTestFramework : ITestFramework, IDataProducer new("TestFails", "TestFails", static () => throw new InvalidOperationException("This test fails on purpose.")), new("TestSkipped", "TestSkipped", Body: null, Skip: true), + + // Deliberately last: a framing desynchronization caused by this node's multi-byte payload + // corrupts whatever the server sends next, so the run-complete handshake is the victim. + new(NonAsciiTestName, NonAsciiTestName, static () => + { + if (Calculator.Add(1, 1) != 2) + { + throw new InvalidOperationException("Add returned the wrong value."); + } + }), ]; public string Uid => nameof(PureTestFramework);