Skip to content

dotnet test MTP: Support live output from test app processes #53495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Youssef1313 wants to merge 2 commits into
base: release/10.0.3xx
Choose a base branch
from
Draft

dotnet test MTP: Support live output from test app processes #53495

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
acc7282
Avoid process output/error redirection completely for MTP on protocol…
Youssef1313 Mar 17, 2026
6086399
Try different approach
Youssef1313 Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Cli/dotnet/Commands/Test/CliConstants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ internal static class HandshakeMessagePropertyNames
internal static class ProtocolConstants
{
/// <summary>
/// The protocol versions that are supported by the current SDK. Multiple versions can be present and be semicolon separated.
/// The protocol versions that are supported by the current SDK. Must be ordered from highest version to lowest version.
/// </summary>
internal const string SupportedVersions = "1.0.0";
internal static readonly string[] SupportedVersions = ["1.1.0", "1.0.0"];
}

internal static class ProjectProperties
Expand Down
2 changes: 1 addition & 1 deletion src/Cli/dotnet/Commands/Test/MTP/Terminal/TerminalTestReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ internal void AssemblyRunCompleted(string executionId,
});
}

internal void HandshakeFailure(string assemblyPath, string? targetFramework, int exitCode, string outputData, string errorData)
internal void HandshakeFailure(string assemblyPath, string? targetFramework, int exitCode, string? outputData, string? errorData)
{
if (_isHelp)
{
Expand Down
112 changes: 62 additions & 50 deletions src/Cli/dotnet/Commands/Test/MTP/TestApplication.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,51 +54,61 @@ public async Task<int> RunAsync()

using var process = Process.Start(processStartInfo)!;

// Reading from process stdout/stderr is done on separate threads to avoid blocking IO on the threadpool.
// Note: even with 'process.StandardOutput.ReadToEndAsync()' or 'process.BeginOutputReadLine()', we ended up with
// many TP threads just doing synchronous IO, slowing down the progress of the test run.
// We want to read requests coming through the pipe and sending responses back to the test app as fast as possible.
// We are using ConcurrentQueue to avoid thread-safety issues for the timeout case.
// In the timeout case, we leave stdOutTask and stdErrTask running, just we stop observing them.
var stdOutBuilder = new ConcurrentQueue<string>();
var stdErrBuilder = new ConcurrentQueue<string>();

var stdOutTask = Task.Factory.StartNew(() =>
int exitCode;
if (!_handler.ShouldRedirectOutputAndError)
{
var stdOut = process.StandardOutput;
string? currentLine;
while ((currentLine = stdOut.ReadLine()) is not null)
await process.WaitForExitAsync();
exitCode = process.ExitCode;
_handler.OnTestProcessExited(exitCode, null, null);
}
else
{
// Reading from process stdout/stderr is done on separate threads to avoid blocking IO on the threadpool.
// Note: even with 'process.StandardOutput.ReadToEndAsync()' or 'process.BeginOutputReadLine()', we ended up with
// many TP threads just doing synchronous IO, slowing down the progress of the test run.
// We want to read requests coming through the pipe and sending responses back to the test app as fast as possible.
// We are using ConcurrentQueue to avoid thread-safety issues for the timeout case.
// In the timeout case, we leave stdOutTask and stdErrTask running, just we stop observing them.
var stdOutBuilder = new ConcurrentQueue<string>();
var stdErrBuilder = new ConcurrentQueue<string>();

var stdOutTask = Task.Factory.StartNew(() =>
{
stdOutBuilder.Enqueue(currentLine);
}
}, TaskCreationOptions.LongRunning);
var stdOut = process.StandardOutput;
string? currentLine;
while ((currentLine = stdOut.ReadLine()) is not null)
{
stdOutBuilder.Enqueue(currentLine);
}
}, TaskCreationOptions.LongRunning);

var stdErrTask = Task.Factory.StartNew(() =>
{
var stdErr = process.StandardError;
string? currentLine;
while ((currentLine = stdErr.ReadLine()) is not null)
var stdErrTask = Task.Factory.StartNew(() =>
{
stdErrBuilder.Enqueue(currentLine);
}
}, TaskCreationOptions.LongRunning);
var stdErr = process.StandardError;
string? currentLine;
while ((currentLine = stdErr.ReadLine()) is not null)
{
stdErrBuilder.Enqueue(currentLine);
}
}, TaskCreationOptions.LongRunning);

// WaitForExitAsync only waits for process exit (and doesn't wait for output) for our usage here.
// If we use BeginOutputReadLine/BeginErrorReadLine, it will also wait for output which can deadlock.
await process.WaitForExitAsync();
// WaitForExitAsync only waits for process exit (and doesn't wait for output) for our usage here.
// If we use BeginOutputReadLine/BeginErrorReadLine, it will also wait for output which can deadlock.
await process.WaitForExitAsync();

// At this point, process already exited. Allow for 5 seconds to consume stdout/stderr.
// We might not be able to consume all the output if the test app has exited but left a child process alive.
try
{
await Task.WhenAll(stdOutTask, stdErrTask).WaitAsync(TimeSpan.FromSeconds(5));
}
catch (TimeoutException)
{
}
// At this point, process already exited. Allow for 5 seconds to consume stdout/stderr.
// We might not be able to consume all the output if the test app has exited but left a child process alive.
try
{
await Task.WhenAll(stdOutTask, stdErrTask).WaitAsync(TimeSpan.FromSeconds(5));
}
catch (TimeoutException)
{
}

var exitCode = process.ExitCode;
_handler.OnTestProcessExited(exitCode, string.Join(Environment.NewLine, stdOutBuilder), string.Join(Environment.NewLine, stdErrBuilder));
exitCode = process.ExitCode;
_handler.OnTestProcessExited(exitCode, string.Join(Environment.NewLine, stdOutBuilder), string.Join(Environment.NewLine, stdErrBuilder));
}

// This condition is to prevent considering the test app as successful when we didn't receive test session end.
// We don't produce the exception if the exit code is already non-zero to avoid surfacing this exception when there is already a known failure.
Expand All @@ -121,15 +131,16 @@ public async Task<int> RunAsync()

private ProcessStartInfo CreateProcessStartInfo()
{
var shouldRedirect = _handler.ShouldRedirectOutputAndError;
Copy link
Member Author

@Youssef1313 Youssef1313 Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot work :/
We haven't started the process, so we don't know the protocol version or anything.

Looks like it's gonna be almost impossible to change this to avoid output/err redirection without being a big breaking change. 😕

var processStartInfo = new ProcessStartInfo
{
// We should get correct RunProperties right away.
// For the case of dotnet test --test-modules path/to/dll, the TestModulesFilterHandler is responsible
// for providing the dotnet muxer as RunCommand, and `exec "path/to/dll"` as RunArguments.
FileName = Module.RunProperties.Command,
Arguments = GetArguments(),
RedirectStandardOutput = true,
RedirectStandardError = true,
RedirectStandardOutput = shouldRedirect,
RedirectStandardError = shouldRedirect,
// False is already the default on .NET Core, but prefer to be explicit.
UseShellExecute = false,
};
Expand Down Expand Up @@ -254,7 +265,7 @@ private Task<IResponse> OnRequest(NamedPipeServer server, IRequest request)
case HandshakeMessage handshakeMessage:
_handshakes.Add(server, handshakeMessage);
string negotiatedVersion = GetSupportedProtocolVersion(handshakeMessage);
OnHandshakeMessage(handshakeMessage, negotiatedVersion.Length > 0);
OnHandshakeMessage(handshakeMessage, negotiatedVersion);
return Task.FromResult((IResponse)CreateHandshakeMessage(negotiatedVersion));

case CommandLineOptionMessages commandLineOptionMessages:
Expand Down Expand Up @@ -317,14 +328,15 @@ private static string GetSupportedProtocolVersion(HandshakeMessage handshakeMess
return string.Empty;
}

// NOTE: Today, ProtocolConstants.Version is only 1.0.0 (i.e, SDK supports only a single version).
// Whenever we support multiple versions in SDK, we should do intersection
// between protocolVersions given by MTP, and the versions supported by SDK.
// Then we return the "highest" version from the intersection.
// The current logic **assumes** that ProtocolConstants.SupportedVersions is a single version.
if (protocolVersions.Split(";").Contains(ProtocolConstants.SupportedVersions))
// ProtocolConstant.SupportedVersions is the SDK supported versions, ordered from highest version to lowest version.
// So, we take the first highest version that is also supported by MTP.
var protocolVersionsByMTP = protocolVersions.Split(";");
foreach (var sdkSupportedVersion in ProtocolConstants.SupportedVersions)
{
return ProtocolConstants.SupportedVersions;
if (protocolVersionsByMTP.Contains(sdkSupportedVersion))
{
return sdkSupportedVersion;
}
}

// The version given by MTP is not supported by SDK.
Expand All @@ -341,8 +353,8 @@ private static HandshakeMessage CreateHandshakeMessage(string version) =>
{ HandshakeMessagePropertyNames.SupportedProtocolVersions, version }
});

public void OnHandshakeMessage(HandshakeMessage handshakeMessage, bool gotSupportedVersion)
=> _handler.OnHandshakeReceived(handshakeMessage, gotSupportedVersion);
public void OnHandshakeMessage(HandshakeMessage handshakeMessage, string negotiatedVersion)
=> _handler.OnHandshakeReceived(handshakeMessage, negotiatedVersion);

private void OnCommandLineOptionMessages(CommandLineOptionMessages commandLineOptionMessages)
{
Expand Down
26 changes: 15 additions & 11 deletions src/Cli/dotnet/Commands/Test/MTP/TestApplicationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ internal sealed class TestApplicationHandler
private readonly Lock _lock = new();
private readonly Dictionary<string, (int TestSessionStartCount, int TestSessionEndCount)> _testSessionEventCountPerSessionUid = new();

private (string? TargetFramework, string? Architecture, string ExecutionId)? _handshakeInfo;
private (string? TargetFramework, string? Architecture, string ExecutionId, string NegotiatedVersion)? _handshakeInfo;
private bool _receivedTestHostHandshake;

public TestApplicationHandler(TerminalTestReporter output, TestModule module, TestOptions options)
Expand All @@ -24,11 +24,14 @@ public TestApplicationHandler(TerminalTestReporter output, TestModule module, Te
_options = options;
}

internal void OnHandshakeReceived(HandshakeMessage handshakeMessage, bool gotSupportedVersion)
// Only 1.0.0 specifically should redirect output/err.
internal bool ShouldRedirectOutputAndError => !_handshakeInfo.HasValue || _handshakeInfo.Value.NegotiatedVersion == "1.0.0";

internal void OnHandshakeReceived(HandshakeMessage handshakeMessage, string negotiatedVersion)
{
LogHandshake(handshakeMessage);

if (!gotSupportedVersion)
if (negotiatedVersion.Length == 0)
{
_output.HandshakeFailure(
_module.TargetPath,
Expand All @@ -37,7 +40,7 @@ internal void OnHandshakeReceived(HandshakeMessage handshakeMessage, bool gotSup
string.Format(
CliCommandStrings.DotnetTestIncompatibleHandshakeVersion,
handshakeMessage.Properties[HandshakeMessagePropertyNames.SupportedProtocolVersions],
ProtocolConstants.SupportedVersions),
string.Join(';', ProtocolConstants.SupportedVersions)),
string.Empty);

// Protocol version is not supported.
Expand All @@ -48,7 +51,7 @@ internal void OnHandshakeReceived(HandshakeMessage handshakeMessage, bool gotSup
var executionId = handshakeMessage.Properties[HandshakeMessagePropertyNames.ExecutionId];
var arch = handshakeMessage.Properties[HandshakeMessagePropertyNames.Architecture]?.ToLower();
var tfm = TargetFrameworkParser.GetShortTargetFramework(handshakeMessage.Properties[HandshakeMessagePropertyNames.Framework]);
var currentHandshakeInfo = (tfm, arch, executionId);
var currentHandshakeInfo = (tfm, arch, executionId, negotiatedVersion);

if (!_handshakeInfo.HasValue)
{
Expand Down Expand Up @@ -137,6 +140,7 @@ internal void OnTestResultsReceived(TestResultMessages testResultMessage)
}

var handshakeInfo = _handshakeInfo.Value;
var shouldRedirect = ShouldRedirectOutputAndError;
foreach (var testResult in testResultMessage.SuccessfulTestMessages)
{
_output.TestCompleted(_module.TargetPath, handshakeInfo.TargetFramework, handshakeInfo.Architecture, handshakeInfo.ExecutionId,
Expand All @@ -149,8 +153,8 @@ internal void OnTestResultsReceived(TestResultMessages testResultMessage)
exceptions: null,
expected: null,
actual: null,
standardOutput: testResult.StandardOutput,
errorOutput: testResult.ErrorOutput);
standardOutput: shouldRedirect ? testResult.StandardOutput : null,
errorOutput: shouldRedirect ? testResult.ErrorOutput : null);
}

foreach (var testResult in testResultMessage.FailedTestMessages)
Expand All @@ -164,8 +168,8 @@ internal void OnTestResultsReceived(TestResultMessages testResultMessage)
exceptions: [.. testResult.Exceptions!.Select(fe => new Terminal.FlatException(fe.ErrorMessage, fe.ErrorType, fe.StackTrace))],
expected: null,
actual: null,
standardOutput: testResult.StandardOutput,
errorOutput: testResult.ErrorOutput);
standardOutput: shouldRedirect ? testResult.StandardOutput : null,
errorOutput: shouldRedirect ? testResult.ErrorOutput : null);
}
}

Expand Down Expand Up @@ -263,7 +267,7 @@ internal bool HasMismatchingTestSessionEventCount()
return false;
}

internal void OnTestProcessExited(int exitCode, string outputData, string errorData)
internal void OnTestProcessExited(int exitCode, string? outputData, string? errorData)
{
if (_receivedTestHostHandshake && _handshakeInfo.HasValue)
{
Expand Down Expand Up @@ -378,7 +382,7 @@ private static void LogFileArtifacts(FileArtifactMessages fileArtifactMessages)
Logger.LogTrace(logMessageBuilder, static logMessageBuilder => logMessageBuilder.ToString());
}

private static void LogTestProcessExit(int exitCode, string outputData, string errorData)
private static void LogTestProcessExit(int exitCode, string? outputData, string? errorData)
{
if (!Logger.TraceEnabled)
{
Expand Down
Loading