From ffd006f6aa24d11877a7870711eddec501fbdaa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 16 Jun 2026 16:29:08 +0200 Subject: [PATCH 1/4] Emit --list-tests json output under --server dotnettestcli (#8661) TerminalOutputDevice short-circuited on _isServerMode in ConsumeAsync and DisplayAfterSessionEndRunAsync, so --list-tests json produced no JSON when launched by 'dotnet test' (which always passes --server dotnettestcli). Both early-returns now allow the JSON discovery path to run, gated on _isListTestsJson; genuine server runs still short-circuit. Adds a black-box acceptance test on top of the #9153 FakeDotnetTestSdk harness validating the JSON document is emitted to stdout under server mode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../OutputDevice/TerminalOutputDevice.cs | 13 +- .../DotnetTestPipeListTestsJsonTests.cs | 141 ++++++++++++++++++ 2 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs index ea2fcba535..9d19c214cb 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs +++ b/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs @@ -455,7 +455,11 @@ public async Task DisplayAfterHotReloadSessionEndAsync(CancellationToken cancell public async Task DisplayAfterSessionEndRunAsync(CancellationToken cancellationToken) { - if (_isServerMode) + // In --list-tests json mode we must still emit the JSON document, even under --server + // (e.g. when launched by `dotnet test` with `--server dotnettestcli`). The JSON is written + // to stdout below, which the dotnet-test SDK captures and forwards. Only the genuine server + // run (no JSON discovery) short-circuits here, because its output flows through the pipe. + if (_isServerMode && !_isListTestsJson) { return; } @@ -588,7 +592,12 @@ public Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationTo { RoslynDebug.Assert(_terminalTestReporter is not null); cancellationToken.ThrowIfCancellationRequested(); - if (_isServerMode) + + // In --list-tests json mode we still need to buffer discovered tests, even under --server + // (e.g. when launched by `dotnet test` with `--server dotnettestcli`), so the JSON document + // can be emitted at the end of the session. Only the genuine server run (no JSON discovery) + // short-circuits here, because its data flows through the pipe instead of the terminal. + if (_isServerMode && !_isListTestsJson) { return Task.CompletedTask; } diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs new file mode 100644 index 0000000000..0473bef2e9 --- /dev/null +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs @@ -0,0 +1,141 @@ +// 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.Text.Json; + +namespace Microsoft.Testing.Platform.Acceptance.IntegrationTests.DotnetTestPipe; + +/// +/// Validates the fix for microsoft/testfx#8661: +/// --list-tests json must still emit its JSON document to stdout when the test app runs under +/// --server dotnettestcli (the mode the .NET SDK always uses for dotnet test). +/// +/// Built on the black-box harness introduced in +/// microsoft/testfx#9153, so the +/// before/after of this behavior is exercised end-to-end against the real wire protocol. +/// +/// +[TestClass] +public class DotnetTestPipeListTestsJsonTests : AcceptanceTestBase +{ + private const string AssetName = "DotnetTestPipeListTestsJson"; + + public TestContext TestContext { get; set; } = null!; + + [TestMethod] + public async Task DotnetTestPipe_ListTestsJson_EmitsJsonDocumentToStdoutUnderServerMode() + { + var testHost = TestInfrastructure.TestHost.LocateFrom( + AssetFixture.TargetAssetPath, AssetName, TargetFrameworks.NetCurrent); + + FakeDotnetTestSdkResult result = await FakeDotnetTestSdk.RunAsync( + testHost, extraArguments: "--list-tests json", cancellationToken: TestContext.CancellationToken); + + result.TestHostResult.AssertExitCodeIs(ExitCode.Success); + + // The JSON document must be the only thing on stdout (no banner, no progress, no summary). + // Trim because the test infrastructure may append a trailing newline. + string output = result.TestHostResult.StandardOutput.Trim(); + Assert.IsTrue( + output.StartsWith('{') && output.EndsWith('}'), + $"Expected stdout to be a single JSON object under --server dotnettestcli, but got:{Environment.NewLine}{output}"); + + Assert.AreEqual( + string.Empty, + result.TestHostResult.StandardError.Trim(), + $"Expected no stderr noise for a successful JSON discovery.{Environment.NewLine}" + + $"Captured stderr:{Environment.NewLine}{result.TestHostResult.StandardError}"); + + using var document = JsonDocument.Parse(output); + JsonElement root = document.RootElement; + + Assert.AreEqual(DiscoveredTestsJsonSchemaVersion, root.GetProperty("schemaVersion").GetInt32()); + + JsonElement tests = root.GetProperty("tests"); + var displayNames = new HashSet(StringComparer.Ordinal); + for (int i = 0; i < tests.GetArrayLength(); i++) + { + displayNames.Add(tests[i].GetProperty("displayName").GetString()!); + } + + Assert.Contains("Test1", displayNames); + Assert.Contains("Test2", displayNames); + } + + /// + /// Mirror of DiscoveredTestsJsonSerializer.SchemaVersion. The serializer type is + /// [Microsoft.CodeAnalysis.Embedded] internal and cannot be referenced from this + /// acceptance project, so the expected schema version is duplicated here intentionally — a + /// bump on the product side should be a conscious update here too. + /// + private const int DiscoveredTestsJsonSchemaVersion = 1; + + public sealed class TestAssetFixture() : TestAssetFixtureBase() + { + private const string AssetCode = """ +#file DotnetTestPipeListTestsJson.csproj + + + $TargetFrameworks$ + enable + enable + Exe + true + preview + + + + + + + +#file Program.cs +using Microsoft.Testing.Platform.Builder; +using Microsoft.Testing.Platform.Capabilities.TestFramework; +using Microsoft.Testing.Platform.Extensions.Messages; +using Microsoft.Testing.Platform.Extensions.TestFramework; + +public class Program +{ + public static async Task Main(string[] args) + { + ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); + builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, __) => new DiscoveringTestFramework()); + using ITestApplication app = await builder.BuildAsync(); + return await app.RunAsync(); + } +} + +public class DiscoveringTestFramework : ITestFramework, IDataProducer +{ + public string Uid => nameof(DiscoveringTestFramework); + public string Version => "2.0.0"; + public string DisplayName => nameof(DiscoveringTestFramework); + public string Description => nameof(DiscoveringTestFramework); + public Type[] DataTypesProduced => new[] { typeof(TestNodeUpdateMessage) }; + public Task IsEnabledAsync() => Task.FromResult(true); + public Task CreateTestSessionAsync(CreateTestSessionContext context) + => Task.FromResult(new CreateTestSessionResult() { IsSuccess = true }); + public Task CloseTestSessionAsync(CloseTestSessionContext context) + => Task.FromResult(new CloseTestSessionResult() { IsSuccess = true }); + + public async Task ExecuteRequestAsync(ExecuteRequestContext context) + { + await context.MessageBus.PublishAsync(this, new TestNodeUpdateMessage(context.Request.Session.SessionUid, + new TestNode() { Uid = "0", DisplayName = "Test1", Properties = new(DiscoveredTestNodeStateProperty.CachedInstance) })); + await context.MessageBus.PublishAsync(this, new TestNodeUpdateMessage(context.Request.Session.SessionUid, + new TestNode() { Uid = "1", DisplayName = "Test2", Properties = new(DiscoveredTestNodeStateProperty.CachedInstance) })); + + context.Complete(); + } +} +"""; + + public string TargetAssetPath => GetAssetPath(AssetName); + + public override (string ID, string Name, string Code) GetAssetsToGenerate() => (AssetName, AssetName, + AssetCode + .PatchTargetFrameworks(TargetFrameworks.NetCurrent) + .PatchCodeWithReplace("$MicrosoftTestingPlatformVersion$", MicrosoftTestingPlatformVersion)); + } +} From 1dc244015ad628c4db487b07c6ca590b01be53f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Wed, 17 Jun 2026 10:17:26 +0200 Subject: [PATCH 2/4] Stream discovered tests to SDK for --list-tests json instead of printing JSON in server mode Per review feedback (Youssef1313): under --server dotnettestcli the test host no longer renders the --list-tests json document itself. Instead it streams the discovered tests to the SDK over the dotnet-test pipe, and the SDK owns producing the JSON (combining tests from every test app into a single document). - TerminalOutputDevice: restore the plain 'if (_isServerMode) return' guards so the terminal device stays silent under --server (standalone --list-tests json is unchanged). - DotnetTestDataConsumer: always stream the full discovery details (file location, method identifier, traits), not only for IDEs, so the SDK has the complete object to build the JSON. - Acceptance test: assert the host keeps stdout/stderr clean under --server and that DiscoveredTestMessages frames carrying the tests are streamed over the pipe. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../OutputDevice/TerminalOutputDevice.cs | 19 +++--- .../DotnetTest/IPC/DotnetTestDataConsumer.cs | 16 ++--- .../DotnetTestPipeListTestsJsonTests.cs | 66 +++++++++---------- .../DotnetTestPipe/DotnetTestPipeProtocol.cs | 52 +++++++++++++++ 4 files changed, 101 insertions(+), 52 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs b/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs index 9d19c214cb..d695be8923 100644 --- a/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs +++ b/src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs @@ -455,11 +455,11 @@ public async Task DisplayAfterHotReloadSessionEndAsync(CancellationToken cancell public async Task DisplayAfterSessionEndRunAsync(CancellationToken cancellationToken) { - // In --list-tests json mode we must still emit the JSON document, even under --server - // (e.g. when launched by `dotnet test` with `--server dotnettestcli`). The JSON is written - // to stdout below, which the dotnet-test SDK captures and forwards. Only the genuine server - // run (no JSON discovery) short-circuits here, because its output flows through the pipe. - if (_isServerMode && !_isListTestsJson) + // Under --server (e.g. `dotnet test` with `--server dotnettestcli`) the terminal device stays + // silent: discovered tests are streamed to the SDK through the dotnet-test pipe (see + // DotnetTestDataConsumer), and the SDK owns rendering — including building the --list-tests json + // document by combining the discovered tests from every test app into a single output. + if (_isServerMode) { return; } @@ -593,11 +593,10 @@ public Task ConsumeAsync(IDataProducer dataProducer, IData value, CancellationTo RoslynDebug.Assert(_terminalTestReporter is not null); cancellationToken.ThrowIfCancellationRequested(); - // In --list-tests json mode we still need to buffer discovered tests, even under --server - // (e.g. when launched by `dotnet test` with `--server dotnettestcli`), so the JSON document - // can be emitted at the end of the session. Only the genuine server run (no JSON discovery) - // short-circuits here, because its data flows through the pipe instead of the terminal. - if (_isServerMode && !_isListTestsJson) + // Under --server (e.g. `dotnet test` with `--server dotnettestcli`) the terminal device does not + // buffer or render anything: data flows to the SDK through the dotnet-test pipe instead, and the + // SDK is responsible for producing the output (including the --list-tests json document). + if (_isServerMode) { return Task.CompletedTask; } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs index 87acb5e7a7..61e128ef69 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs @@ -54,15 +54,13 @@ public async Task ConsumeAsync(IDataProducer dataProducer, IData value, Cancella switch (testNodeDetails.State) { case TestStates.Discovered: - TestFileLocationProperty? testFileLocationProperty = null; - TestMethodIdentifierProperty? testMethodIdentifierProperty = null; - TestMetadataProperty[] traits = []; - if (_dotnetTestConnection.IsIDE) - { - testFileLocationProperty = testNodeUpdateMessage.TestNode.Properties.SingleOrDefault(); - testMethodIdentifierProperty = testNodeUpdateMessage.TestNode.Properties.SingleOrDefault(); - traits = testNodeUpdateMessage.TestNode.Properties.OfType(); - } + // Always stream the full discovery details (file location, method identifier, + // traits) to the SDK — not only for IDEs. `dotnet test --list-tests json` is + // produced on the SDK side by combining the discovered tests from every test + // app into a single document, so the SDK needs the complete object here. + TestFileLocationProperty? testFileLocationProperty = testNodeUpdateMessage.TestNode.Properties.SingleOrDefault(); + TestMethodIdentifierProperty? testMethodIdentifierProperty = testNodeUpdateMessage.TestNode.Properties.SingleOrDefault(); + TestMetadataProperty[] traits = testNodeUpdateMessage.TestNode.Properties.OfType(); DiscoveredTestMessages discoveredTestMessages = new( _executionId, diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs index 0473bef2e9..f9cbd4aebc 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs @@ -1,18 +1,18 @@ // 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.Text.Json; - namespace Microsoft.Testing.Platform.Acceptance.IntegrationTests.DotnetTestPipe; /// -/// Validates the fix for microsoft/testfx#8661: -/// --list-tests json must still emit its JSON document to stdout when the test app runs under -/// --server dotnettestcli (the mode the .NET SDK always uses for dotnet test). +/// Validates the agreed design for --list-tests json under --server dotnettestcli +/// (the mode the .NET SDK always uses for dotnet test): the test app does not render +/// the JSON document itself. Instead it streams the discovered tests to the SDK over the dotnet-test +/// pipe, and the SDK is responsible for producing the JSON (combining the tests from every test app +/// into a single document). /// /// Built on the black-box harness introduced in /// microsoft/testfx#9153, so the -/// before/after of this behavior is exercised end-to-end against the real wire protocol. +/// behavior is exercised end-to-end against the real wire protocol. /// /// [TestClass] @@ -23,7 +23,7 @@ public class DotnetTestPipeListTestsJsonTests : AcceptanceTestBase(StringComparer.Ordinal); - for (int i = 0; i < tests.GetArrayLength(); i++) + // The discovered tests must instead be streamed to the SDK as DiscoveredTestMessages frames. + var discoveredDisplayNames = new HashSet(StringComparer.Ordinal); + bool sawDiscoveredFrame = false; + foreach (RawMessage frame in result.ReceivedMessages) { - displayNames.Add(tests[i].GetProperty("displayName").GetString()!); + if (frame.SerializerId != DotnetTestPipeProtocol.SerializerIds.DiscoveredTestMessages) + { + continue; + } + + sawDiscoveredFrame = true; + foreach (string displayName in DotnetTestPipeProtocol.DecodeDiscoveredTestDisplayNames(frame.Body)) + { + discoveredDisplayNames.Add(displayName); + } } - Assert.Contains("Test1", displayNames); - Assert.Contains("Test2", displayNames); + Assert.IsTrue(sawDiscoveredFrame, "Expected at least one DiscoveredTestMessages frame over the dotnet-test pipe."); + Assert.Contains("Test1", discoveredDisplayNames); + Assert.Contains("Test2", discoveredDisplayNames); } - /// - /// Mirror of DiscoveredTestsJsonSerializer.SchemaVersion. The serializer type is - /// [Microsoft.CodeAnalysis.Embedded] internal and cannot be referenced from this - /// acceptance project, so the expected schema version is duplicated here intentionally — a - /// bump on the product side should be a conscious update here too. - /// - private const int DiscoveredTestsJsonSchemaVersion = 1; - public sealed class TestAssetFixture() : TestAssetFixtureBase() { private const string AssetCode = """ diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeProtocol.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeProtocol.cs index f6ca6151e0..31fd284d99 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeProtocol.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeProtocol.cs @@ -220,6 +220,58 @@ public static (byte? SessionType, string? SessionUid, string? ExecutionId) Decod return (sessionType, sessionUid, executionId); } + /// + /// Decodes the display names carried by a + /// frame. Mirrors the wire layout produced by DiscoveredTestMessagesSerializer: the body + /// is a field-tagged object whose DiscoveredTestMessageList field (id 3) holds a + /// length-prefixed array of field-tagged test messages, each carrying a DisplayName + /// field (id 2). Unknown fields are skipped via their declared size, exactly like the product + /// deserializer. + /// + public static IReadOnlyList DecodeDiscoveredTestDisplayNames(byte[] body) + { + const ushort discoveredTestMessageListFieldId = 3; + const ushort displayNameFieldId = 2; + + List displayNames = []; + using MemoryStream stream = new(body, writable: false); + + ushort fieldCount = ReadUShort(stream); + for (int i = 0; i < fieldCount; i++) + { + ushort fieldId = ReadUShort(stream); + int fieldSize = ReadInt(stream); + + if (fieldId != discoveredTestMessageListFieldId) + { + // ExecutionId / InstanceId (or any future field): skip the whole payload. + stream.Seek(fieldSize, SeekOrigin.Current); + continue; + } + + int messageCount = ReadInt(stream); + for (int m = 0; m < messageCount; m++) + { + ushort messageFieldCount = ReadUShort(stream); + for (int f = 0; f < messageFieldCount; f++) + { + ushort messageFieldId = ReadUShort(stream); + int messageFieldSize = ReadInt(stream); + if (messageFieldId == displayNameFieldId) + { + displayNames.Add(ReadFixedSizeString(stream, messageFieldSize)); + } + else + { + stream.Seek(messageFieldSize, SeekOrigin.Current); + } + } + } + } + + return displayNames; + } + private static async Task TryReadExactlyAsync(Stream stream, Memory buffer, CancellationToken cancellationToken) { int totalRead = 0; From 480a169d72b0a926a89db58cf1fed484cd671577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Wed, 17 Jun 2026 11:33:09 +0200 Subject: [PATCH 3/4] Gate streamed discovery details on IsIDE instead of always sending them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review feedback (Youssef1313): rather than always streaming the full discovery details (file location, method identifier, traits), only send them when the consumer asks for them. Reuse the existing IsIDE handshake flag for that — the SDK sets it when it wants the complete discovery object (e.g. for dotnet test --list-tests json). Plain runs keep the DiscoveredTestMessages payload minimal (Uid + DisplayName, as before). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DotnetTest/IPC/DotnetTestDataConsumer.cs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs index 61e128ef69..8046483788 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/DotnetTestDataConsumer.cs @@ -54,13 +54,20 @@ public async Task ConsumeAsync(IDataProducer dataProducer, IData value, Cancella switch (testNodeDetails.State) { case TestStates.Discovered: - // Always stream the full discovery details (file location, method identifier, - // traits) to the SDK — not only for IDEs. `dotnet test --list-tests json` is - // produced on the SDK side by combining the discovered tests from every test - // app into a single document, so the SDK needs the complete object here. - TestFileLocationProperty? testFileLocationProperty = testNodeUpdateMessage.TestNode.Properties.SingleOrDefault(); - TestMethodIdentifierProperty? testMethodIdentifierProperty = testNodeUpdateMessage.TestNode.Properties.SingleOrDefault(); - TestMetadataProperty[] traits = testNodeUpdateMessage.TestNode.Properties.OfType(); + // Only stream the full discovery details (file location, method identifier, + // traits) when the consumer asked for them. We reuse the existing IsIDE flag + // for that — despite the name, it is the handshake signal a consumer sets when + // it wants the complete discovery object (e.g. an IDE, or the SDK when running + // `dotnet test --list-tests json`). Plain runs keep the payload minimal. + TestFileLocationProperty? testFileLocationProperty = null; + TestMethodIdentifierProperty? testMethodIdentifierProperty = null; + TestMetadataProperty[] traits = []; + if (_dotnetTestConnection.IsIDE) + { + testFileLocationProperty = testNodeUpdateMessage.TestNode.Properties.SingleOrDefault(); + testMethodIdentifierProperty = testNodeUpdateMessage.TestNode.Properties.SingleOrDefault(); + traits = testNodeUpdateMessage.TestNode.Properties.OfType(); + } DiscoveredTestMessages discoveredTestMessages = new( _executionId, From 58bec7b157949edd10aea68d06b33cc178c42b07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Wed, 17 Jun 2026 11:55:14 +0200 Subject: [PATCH 4/4] Assert full discovery metadata flows over dotnet-test pipe (--list-tests json) Address review feedback on #9192: - Run the acceptance test with isIde: true so the host streams the full discovery object (file location, method identifier, traits), matching the intended dotnet test --list-tests json scenario. - Enrich the test asset's test nodes with TestFileLocationProperty, TestMethodIdentifierProperty and TestMetadataProperty, and decode + assert those fields over the pipe so regressions where the SDK-facing metadata stops flowing are caught. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DotnetTestPipeListTestsJsonTests.cs | 57 ++++++- .../DotnetTestPipe/DotnetTestPipeProtocol.cs | 157 ++++++++++++++++-- 2 files changed, 190 insertions(+), 24 deletions(-) diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs index f9cbd4aebc..0740e172dd 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeListTestsJsonTests.cs @@ -28,8 +28,11 @@ public async Task DotnetTestPipe_ListTestsJson_StreamsDiscoveredTestsOverPipeAnd var testHost = TestInfrastructure.TestHost.LocateFrom( AssetFixture.TargetAssetPath, AssetName, TargetFrameworks.NetCurrent); + // The SDK requests the full discovery object by advertising IsIDE in the handshake (the same + // signal it sets when running `dotnet test --list-tests json`), so the host streams the + // complete discovery details (file location, method identifier, traits) we assert below. FakeDotnetTestSdkResult result = await FakeDotnetTestSdk.RunAsync( - testHost, extraArguments: "--list-tests json", cancellationToken: TestContext.CancellationToken); + testHost, extraArguments: "--list-tests json", isIde: true, cancellationToken: TestContext.CancellationToken); result.TestHostResult.AssertExitCodeIs(ExitCode.Success); @@ -48,8 +51,10 @@ public async Task DotnetTestPipe_ListTestsJson_StreamsDiscoveredTestsOverPipeAnd $"Expected no stderr noise for a successful discovery.{Environment.NewLine}" + $"Captured stderr:{Environment.NewLine}{result.TestHostResult.StandardError}"); - // The discovered tests must instead be streamed to the SDK as DiscoveredTestMessages frames. - var discoveredDisplayNames = new HashSet(StringComparer.Ordinal); + // The discovered tests must instead be streamed to the SDK as DiscoveredTestMessages frames, + // carrying the full discovery object (file location, method identifier, traits) the SDK needs + // to build the --list-tests json document. + var discoveredTests = new Dictionary(StringComparer.Ordinal); bool sawDiscoveredFrame = false; foreach (RawMessage frame in result.ReceivedMessages) { @@ -59,15 +64,31 @@ public async Task DotnetTestPipe_ListTestsJson_StreamsDiscoveredTestsOverPipeAnd } sawDiscoveredFrame = true; - foreach (string displayName in DotnetTestPipeProtocol.DecodeDiscoveredTestDisplayNames(frame.Body)) + foreach (DiscoveredTest test in DotnetTestPipeProtocol.DecodeDiscoveredTests(frame.Body)) { - discoveredDisplayNames.Add(displayName); + Assert.IsNotNull(test.DisplayName, "Every discovered test must carry a display name."); + discoveredTests[test.DisplayName] = test; } } Assert.IsTrue(sawDiscoveredFrame, "Expected at least one DiscoveredTestMessages frame over the dotnet-test pipe."); - Assert.Contains("Test1", discoveredDisplayNames); - Assert.Contains("Test2", discoveredDisplayNames); + Assert.Contains("Test1", discoveredTests.Keys); + Assert.Contains("Test2", discoveredTests.Keys); + + // Regression guard: the full discovery details must keep flowing over the pipe so the SDK can + // build the --list-tests json document. If any of these fields stop being streamed the test + // fails, even though the display names alone would still arrive. + DiscoveredTest test1 = discoveredTests["Test1"]; + Assert.AreEqual("MyTests.cs", test1.FilePath); + Assert.AreEqual("MyNamespace", test1.Namespace); + Assert.AreEqual("MyTestClass", test1.TypeName); + Assert.AreEqual("Test1", test1.MethodName); + Assert.AreEqual("Smoke", test1.Traits["Category"]); + + DiscoveredTest test2 = discoveredTests["Test2"]; + Assert.AreEqual("MyTests.cs", test2.FilePath); + Assert.AreEqual("Test2", test2.MethodName); + Assert.AreEqual("Integration", test2.Traits["Category"]); } public sealed class TestAssetFixture() : TestAssetFixtureBase() @@ -122,9 +143,27 @@ public Task CloseTestSessionAsync(CloseTestSessionContex public async Task ExecuteRequestAsync(ExecuteRequestContext context) { await context.MessageBus.PublishAsync(this, new TestNodeUpdateMessage(context.Request.Session.SessionUid, - new TestNode() { Uid = "0", DisplayName = "Test1", Properties = new(DiscoveredTestNodeStateProperty.CachedInstance) })); + new TestNode() + { + Uid = "0", + DisplayName = "Test1", + Properties = new( + DiscoveredTestNodeStateProperty.CachedInstance, + new TestFileLocationProperty("MyTests.cs", new LinePositionSpan(new LinePosition(10, 1), new LinePosition(10, 5))), + new TestMethodIdentifierProperty("MyTestAssembly", "MyNamespace", "MyTestClass", "Test1", 0, [], "System.Void"), + new TestMetadataProperty("Category", "Smoke")), + })); await context.MessageBus.PublishAsync(this, new TestNodeUpdateMessage(context.Request.Session.SessionUid, - new TestNode() { Uid = "1", DisplayName = "Test2", Properties = new(DiscoveredTestNodeStateProperty.CachedInstance) })); + new TestNode() + { + Uid = "1", + DisplayName = "Test2", + Properties = new( + DiscoveredTestNodeStateProperty.CachedInstance, + new TestFileLocationProperty("MyTests.cs", new LinePositionSpan(new LinePosition(20, 1), new LinePosition(20, 5))), + new TestMethodIdentifierProperty("MyTestAssembly", "MyNamespace", "MyTestClass", "Test2", 0, [], "System.Void"), + new TestMetadataProperty("Category", "Integration")), + })); context.Complete(); } diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeProtocol.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeProtocol.cs index 31fd284d99..b120d5c9e1 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeProtocol.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeProtocol.cs @@ -221,19 +221,33 @@ public static (byte? SessionType, string? SessionUid, string? ExecutionId) Decod } /// - /// Decodes the display names carried by a - /// frame. Mirrors the wire layout produced by DiscoveredTestMessagesSerializer: the body - /// is a field-tagged object whose DiscoveredTestMessageList field (id 3) holds a - /// length-prefixed array of field-tagged test messages, each carrying a DisplayName - /// field (id 2). Unknown fields are skipped via their declared size, exactly like the product + /// Decodes the tests carried by a frame, + /// including the full discovery details (file path, line number, namespace, type/method name, + /// parameter types and traits). Mirrors the wire layout produced by + /// DiscoveredTestMessagesSerializer: the body is a field-tagged object whose + /// DiscoveredTestMessageList field (id 3) holds a length-prefixed array of field-tagged + /// test messages. Unknown fields are skipped via their declared size, exactly like the product /// deserializer. + /// + /// Decoding the whole object (not just the display name) lets the acceptance test catch + /// regressions where the SDK-facing metadata required to build the --list-tests json + /// document stops flowing over the pipe. + /// /// - public static IReadOnlyList DecodeDiscoveredTestDisplayNames(byte[] body) + public static IReadOnlyList DecodeDiscoveredTests(byte[] body) { const ushort discoveredTestMessageListFieldId = 3; + const ushort uidFieldId = 1; const ushort displayNameFieldId = 2; - - List displayNames = []; + const ushort filePathFieldId = 3; + const ushort lineNumberFieldId = 4; + const ushort namespaceFieldId = 5; + const ushort typeNameFieldId = 6; + const ushort methodNameFieldId = 7; + const ushort traitsFieldId = 8; + const ushort parameterTypeFullNamesFieldId = 9; + + List discoveredTests = []; using MemoryStream stream = new(body, writable: false); ushort fieldCount = ReadUShort(stream); @@ -252,24 +266,122 @@ public static IReadOnlyList DecodeDiscoveredTestDisplayNames(byte[] body int messageCount = ReadInt(stream); for (int m = 0; m < messageCount; m++) { + string? uid = null; + string? displayName = null; + string? filePath = null; + int? lineNumber = null; + string? @namespace = null; + string? typeName = null; + string? methodName = null; + string[] parameterTypeFullNames = []; + Dictionary traits = []; + ushort messageFieldCount = ReadUShort(stream); for (int f = 0; f < messageFieldCount; f++) { ushort messageFieldId = ReadUShort(stream); int messageFieldSize = ReadInt(stream); - if (messageFieldId == displayNameFieldId) - { - displayNames.Add(ReadFixedSizeString(stream, messageFieldSize)); - } - else + switch (messageFieldId) { - stream.Seek(messageFieldSize, SeekOrigin.Current); + case uidFieldId: + uid = ReadFixedSizeString(stream, messageFieldSize); + break; + case displayNameFieldId: + displayName = ReadFixedSizeString(stream, messageFieldSize); + break; + case filePathFieldId: + filePath = ReadFixedSizeString(stream, messageFieldSize); + break; + case lineNumberFieldId: + lineNumber = ReadInt(stream); + break; + case namespaceFieldId: + @namespace = ReadFixedSizeString(stream, messageFieldSize); + break; + case typeNameFieldId: + typeName = ReadFixedSizeString(stream, messageFieldSize); + break; + case methodNameFieldId: + methodName = ReadFixedSizeString(stream, messageFieldSize); + break; + case traitsFieldId: + foreach (KeyValuePair trait in ReadTraits(stream)) + { + traits[trait.Key] = trait.Value; + } + + break; + case parameterTypeFullNamesFieldId: + parameterTypeFullNames = ReadParameterTypeFullNames(stream); + break; + default: + stream.Seek(messageFieldSize, SeekOrigin.Current); + break; } } + + discoveredTests.Add(new DiscoveredTest( + uid, displayName, filePath, lineNumber, @namespace, typeName, methodName, parameterTypeFullNames, traits)); + } + } + + return discoveredTests; + } + + /// + /// Reads the Traits payload (id 8) of a discovered test message: a length-prefixed array + /// of field-tagged key/value pairs (Key id 1, Value id 2). + /// + private static IReadOnlyList> ReadTraits(Stream stream) + { + const ushort keyFieldId = 1; + const ushort valueFieldId = 2; + + int length = ReadInt(stream); + List> traits = []; + for (int i = 0; i < length; i++) + { + string? key = null; + string? value = null; + ushort fieldCount = ReadUShort(stream); + for (int f = 0; f < fieldCount; f++) + { + ushort fieldId = ReadUShort(stream); + int fieldSize = ReadInt(stream); + switch (fieldId) + { + case keyFieldId: + key = ReadFixedSizeString(stream, fieldSize); + break; + case valueFieldId: + value = ReadFixedSizeString(stream, fieldSize); + break; + default: + stream.Seek(fieldSize, SeekOrigin.Current); + break; + } } + + traits.Add(new KeyValuePair(key ?? string.Empty, value ?? string.Empty)); + } + + return traits; + } + + /// + /// Reads the ParameterTypeFullNames payload (id 9) of a discovered test message: a + /// length-prefixed array of length-prefixed UTF-8 strings. + /// + private static string[] ReadParameterTypeFullNames(Stream stream) + { + int length = ReadInt(stream); + string[] parameterTypeFullNames = new string[length]; + for (int i = 0; i < length; i++) + { + parameterTypeFullNames[i] = ReadLengthPrefixedString(stream); } - return displayNames; + return parameterTypeFullNames; } private static async Task TryReadExactlyAsync(Stream stream, Memory buffer, CancellationToken cancellationToken) @@ -351,3 +463,18 @@ private static void WriteLengthPrefixedString(Stream stream, string value) /// A raw decoded pipe frame (serializer id + body bytes, no further decoding). internal sealed record RawMessage(int SerializerId, byte[] Body); + +/// +/// A fully decoded discovered test message: the complete discovery object the SDK needs to build the +/// --list-tests json document (display name plus file/method location and traits). +/// +internal sealed record DiscoveredTest( + string? Uid, + string? DisplayName, + string? FilePath, + int? LineNumber, + string? Namespace, + string? TypeName, + string? MethodName, + string[] ParameterTypeFullNames, + IReadOnlyDictionary Traits);