Skip to content

Commit 960ad67

Browse files
Youssef1313Youssef1313
authored
Cleanup dotnet test for MTP (#50625)
1 parent eeebc2d commit 960ad67

File tree

8 files changed

+164
-276
lines changed

8 files changed

+164
-276
lines changed

‎src/Cli/dotnet/Commands/Test/CustomEventArgs.cs‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -5,61 +5,9 @@
55

66
namespace Microsoft.DotNet.Cli.Commands.Test;
77

8-
internal sealed class HandshakeArgs : EventArgs
9-
{
10-
public Handshake Handshake { get; set; }
11-
public bool GotSupportedVersion { get; set; }
12-
}
13-
148
internal sealed class HelpEventArgs : EventArgs
159
{
1610
public string ModulePath { get; set; }
1711

1812
public CommandLineOption[] CommandLineOptions { get; set; }
1913
}
20-
21-
internal sealed class DiscoveredTestEventArgs : EventArgs
22-
{
23-
public string ExecutionId { get; set; }
24-
25-
public string InstanceId { get; set; }
26-
27-
public DiscoveredTest[] DiscoveredTests { get; set; }
28-
}
29-
30-
internal sealed class TestResultEventArgs : EventArgs
31-
{
32-
public string ExecutionId { get; set; }
33-
34-
public string InstanceId { get; set; }
35-
36-
public SuccessfulTestResult[] SuccessfulTestResults { get; set; }
37-
38-
public FailedTestResult[] FailedTestResults { get; set; }
39-
}
40-
41-
internal sealed class FileArtifactEventArgs : EventArgs
42-
{
43-
public string ExecutionId { get; set; }
44-
45-
public string InstanceId { get; set; }
46-
47-
public FileArtifact[] FileArtifacts { get; set; }
48-
}
49-
50-
internal sealed class SessionEventArgs : EventArgs
51-
{
52-
public TestSession SessionEvent { get; set; }
53-
}
54-
55-
internal sealed class ErrorEventArgs : EventArgs
56-
{
57-
public string ErrorMessage { get; set; }
58-
}
59-
60-
internal sealed class TestProcessExitEventArgs : EventArgs
61-
{
62-
public List<string> OutputData { get; set; }
63-
public List<string> ErrorData { get; set; }
64-
public int ExitCode { get; set; }
65-
}

‎src/Cli/dotnet/Commands/Test/Models.cs‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -114,18 +114,4 @@ public bool MoveNext()
114114

115115
internal sealed record TestModule(RunProperties RunProperties, string? ProjectFullPath, string? TargetFramework, bool IsTestingPlatformApplication, bool IsTestProject, ProjectLaunchSettingsModel? LaunchSettings, string TargetPath, string? DotnetRootArchVariableName);
116116

117-
internal sealed record Handshake(Dictionary<byte, string>? Properties);
118-
119117
internal sealed record CommandLineOption(string Name, string Description, bool? IsHidden, bool? IsBuiltIn);
120-
121-
internal sealed record DiscoveredTest(string? Uid, string? DisplayName);
122-
123-
internal sealed record SuccessfulTestResult(string? Uid, string? DisplayName, byte? State, long? Duration, string? Reason, string? StandardOutput, string? ErrorOutput, string? SessionUid);
124-
125-
internal sealed record FailedTestResult(string? Uid, string? DisplayName, byte? State, long? Duration, string? Reason, FlatException[]? Exceptions, string? StandardOutput, string? ErrorOutput, string? SessionUid);
126-
127-
internal sealed record FlatException(string? ErrorMessage, string? ErrorType, string? StackTrace);
128-
129-
internal sealed record FileArtifact(string? FullPath, string? DisplayName, string? Description, string? TestUid, string? TestDisplayName, string? SessionUid);
130-
131-
internal sealed record TestSession(byte? SessionType, string? SessionUid, string? ExecutionId);

‎src/Cli/dotnet/Commands/Test/Terminal/TerminalTestReporter.cs‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ private void AppendTestRunSummary(ITerminal terminal, int? exitCode)
210210
{
211211
terminal.Append(DoubleIndentation);
212212
terminal.Append("- ");
213-
if (!String.IsNullOrWhiteSpace(artifact.TestName))
213+
if (!string.IsNullOrWhiteSpace(artifact.TestName))
214214
{
215215
terminal.Append(CliCommandStrings.ForTest);
216216
terminal.Append(" '");
@@ -364,7 +364,7 @@ private void AppendTestRunSummary(ITerminal terminal, int? exitCode)
364364
AppendExitCodeAndUrl(terminal, exitCode, isRun: true);
365365
}
366366

367-
private void AppendExitCodeAndUrl(ITerminal terminal, int? exitCode, bool isRun)
367+
private static void AppendExitCodeAndUrl(ITerminal terminal, int? exitCode, bool isRun)
368368
{
369369
// When we crash with exception we don't have any predetermined exit code, and won't write our helper message to point users to exit code overview.
370370
// When we succeed we also don't point users to exit code overview.
@@ -568,7 +568,7 @@ private static void FormatErrorMessage(ITerminal terminal, FlatException[] excep
568568
string? firstErrorMessage = GetStringFromIndexOrDefault(exceptions, e => e.ErrorMessage, index);
569569
string? firstErrorType = GetStringFromIndexOrDefault(exceptions, e => e.ErrorType, index);
570570
string? firstStackTrace = GetStringFromIndexOrDefault(exceptions, e => e.StackTrace, index);
571-
if (String.IsNullOrWhiteSpace(firstErrorMessage) && String.IsNullOrWhiteSpace(firstErrorType) && String.IsNullOrWhiteSpace(firstStackTrace))
571+
if (string.IsNullOrWhiteSpace(firstErrorMessage) && string.IsNullOrWhiteSpace(firstErrorType) && string.IsNullOrWhiteSpace(firstStackTrace))
572572
{
573573
return;
574574
}
@@ -597,7 +597,7 @@ private static void FormatErrorMessage(ITerminal terminal, FlatException[] excep
597597

598598
private static void FormatExpectedAndActual(ITerminal terminal, string? expected, string? actual)
599599
{
600-
if (String.IsNullOrWhiteSpace(expected) && String.IsNullOrWhiteSpace(actual))
600+
if (string.IsNullOrWhiteSpace(expected) && string.IsNullOrWhiteSpace(actual))
601601
{
602602
return;
603603
}
@@ -615,7 +615,7 @@ private static void FormatExpectedAndActual(ITerminal terminal, string? expected
615615
private static void FormatStackTrace(ITerminal terminal, FlatException[] exceptions, int index)
616616
{
617617
string? stackTrace = GetStringFromIndexOrDefault(exceptions, e => e.StackTrace, index);
618-
if (String.IsNullOrWhiteSpace(stackTrace))
618+
if (string.IsNullOrWhiteSpace(stackTrace))
619619
{
620620
return;
621621
}
@@ -633,7 +633,7 @@ private static void FormatStackTrace(ITerminal terminal, FlatException[] excepti
633633

634634
private static void FormatStandardAndErrorOutput(ITerminal terminal, string? standardOutput, string? standardError)
635635
{
636-
if (String.IsNullOrWhiteSpace(standardOutput) && String.IsNullOrWhiteSpace(standardError))
636+
if (string.IsNullOrWhiteSpace(standardOutput) && string.IsNullOrWhiteSpace(standardError))
637637
{
638638
return;
639639
}
@@ -680,7 +680,7 @@ private static void AppendAssemblyLinkTargetFrameworkAndArchitecture(ITerminal t
680680
Match match = GetFrameRegex().Match(stackTraceLine);
681681
if (match.Success)
682682
{
683-
bool weHaveFilePathAndCodeLine = !String.IsNullOrWhiteSpace(match.Groups["code"].Value);
683+
bool weHaveFilePathAndCodeLine = !string.IsNullOrWhiteSpace(match.Groups["code"].Value);
684684
terminal.Append(CliCommandStrings.StackFrameAt);
685685
terminal.Append(' ');
686686

@@ -698,7 +698,7 @@ private static void AppendAssemblyLinkTargetFrameworkAndArchitecture(ITerminal t
698698
terminal.Append(' ');
699699
terminal.Append(CliCommandStrings.StackFrameIn);
700700
terminal.Append(' ');
701-
if (!String.IsNullOrWhiteSpace(match.Groups["file"].Value))
701+
if (!string.IsNullOrWhiteSpace(match.Groups["file"].Value))
702702
{
703703
int line = int.TryParse(match.Groups["line"].Value, out int value) ? value : 0;
704704
terminal.AppendLink(match.Groups["file"].Value, line);
@@ -718,7 +718,7 @@ private static void AppendAssemblyLinkTargetFrameworkAndArchitecture(ITerminal t
718718

719719
private static void AppendIndentedLine(ITerminal terminal, string? message, string indent)
720720
{
721-
if (String.IsNullOrWhiteSpace(message))
721+
if (string.IsNullOrWhiteSpace(message))
722722
{
723723
return;
724724
}
@@ -904,9 +904,6 @@ public void WriteMessage(string text, SystemConsoleColor? color = null, int? pad
904904
}
905905

906906
internal void TestDiscovered(
907-
string assembly,
908-
string? targetFramework,
909-
string? architecture,
910907
string executionId,
911908
string? displayName,
912909
string? uid)
@@ -1001,11 +998,7 @@ private static TerminalColor ToTerminalColor(ConsoleColor consoleColor)
1001998
};
1002999

10031000
public void TestInProgress(
1004-
string assembly,
1005-
string? targetFramework,
1006-
string? architecture,
10071001
string testNodeUid,
1008-
string instanceId,
10091002
string displayName,
10101003
string executionId)
10111004
{

‎src/Cli/dotnet/Commands/Test/TestApplication.cs‎

Lines changed: 34 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,24 @@
44
#nullable disable
55

66
using System.Diagnostics;
7+
using System.Globalization;
78
using System.IO.Pipes;
89
using Microsoft.DotNet.Cli.Commands.Test.IPC;
910
using Microsoft.DotNet.Cli.Commands.Test.IPC.Models;
1011
using Microsoft.DotNet.Cli.Commands.Test.IPC.Serializers;
12+
using Microsoft.DotNet.Cli.Commands.Test.Terminal;
1113
using Microsoft.DotNet.Cli.Utils;
1214

1315
namespace Microsoft.DotNet.Cli.Commands.Test;
1416

15-
internal sealed class TestApplication(TestModule module, BuildOptions buildOptions) : IDisposable
17+
internal sealed class TestApplication(
18+
TestModule module,
19+
BuildOptions buildOptions,
20+
TestOptions testOptions,
21+
TerminalTestReporter output) : IDisposable
1622
{
1723
private readonly BuildOptions _buildOptions = buildOptions;
24+
private readonly TestApplicationHandler _handler = new(output, module, testOptions);
1825

1926
private readonly List<string> _outputData = [];
2027
private readonly List<string> _errorData = [];
@@ -25,27 +32,23 @@ internal sealed class TestApplication(TestModule module, BuildOptions buildOptio
2532
private readonly List<NamedPipeServer> _testAppPipeConnections = [];
2633
private readonly Dictionary<NamedPipeServer, HandshakeMessage> _handshakes = new();
2734

28-
public event EventHandler<HandshakeArgs> HandshakeReceived;
2935
public event EventHandler<HelpEventArgs> HelpRequested;
30-
public event EventHandler<DiscoveredTestEventArgs> DiscoveredTestsReceived;
31-
public event EventHandler<TestResultEventArgs> TestResultsReceived;
32-
public event EventHandler<FileArtifactEventArgs> FileArtifactsReceived;
33-
public event EventHandler<SessionEventArgs> SessionEventReceived;
34-
public event EventHandler<ErrorEventArgs> ErrorReceived;
35-
public event EventHandler<TestProcessExitEventArgs> TestProcessExited;
3636

3737
public TestModule Module { get; } = module;
38+
public TestOptions TestOptions { get; } = testOptions;
3839

3940
public bool HasFailureDuringDispose { get; private set; }
4041

41-
public async Task<int> RunAsync(TestOptions testOptions)
42+
public async Task<int> RunAsync()
4243
{
43-
if (testOptions.HasFilterMode && !ModulePathExists())
44+
// TODO: RunAsync is probably expected to be executed exactly once on each TestApplication instance.
45+
// Consider throwing an exception if it's called more than once.
46+
if (TestOptions.HasFilterMode && !ModulePathExists())
4447
{
4548
return ExitCode.GenericFailure;
4649
}
4750

48-
var processStartInfo = CreateProcessStartInfo(testOptions);
51+
var processStartInfo = CreateProcessStartInfo();
4952

5053
_testAppPipeConnectionLoop = Task.Run(async () => await WaitConnectionAsync(_cancellationToken.Token), _cancellationToken.Token);
5154
var testProcessResult = await StartProcess(processStartInfo);
@@ -55,15 +58,15 @@ public async Task<int> RunAsync(TestOptions testOptions)
5558
return testProcessResult;
5659
}
5760

58-
private ProcessStartInfo CreateProcessStartInfo(TestOptions testOptions)
61+
private ProcessStartInfo CreateProcessStartInfo()
5962
{
6063
var processStartInfo = new ProcessStartInfo
6164
{
6265
// We should get correct RunProperties right away.
6366
// For the case of dotnet test --test-modules path/to/dll, the TestModulesFilterHandler is responsible
6467
// for providing the dotnet muxer as RunCommand, and `exec "path/to/dll"` as RunArguments.
6568
FileName = Module.RunProperties.Command,
66-
Arguments = GetArguments(testOptions),
69+
Arguments = GetArguments(),
6770
RedirectStandardOutput = true,
6871
RedirectStandardError = true,
6972
};
@@ -96,7 +99,7 @@ private ProcessStartInfo CreateProcessStartInfo(TestOptions testOptions)
9699
return processStartInfo;
97100
}
98101

99-
private string GetArguments(TestOptions testOptions)
102+
private string GetArguments()
100103
{
101104
// Keep RunArguments first.
102105
// In the case of UseAppHost=false, RunArguments is set to `exec $(TargetPath)`:
@@ -107,7 +110,7 @@ private string GetArguments(TestOptions testOptions)
107110
// In short, it's expected to already be escaped properly.
108111
StringBuilder builder = new(Module.RunProperties.Arguments);
109112

110-
if (testOptions.IsHelp)
113+
if (TestOptions.IsHelp)
111114
{
112115
builder.Append($" {TestingPlatformOptions.HelpOption.Name}");
113116
}
@@ -259,9 +262,9 @@ private static string GetSupportedProtocolVersion(HandshakeMessage handshakeMess
259262
}
260263

261264
private static HandshakeMessage CreateHandshakeMessage(string version) =>
262-
new(new Dictionary<byte, string>
265+
new(new Dictionary<byte, string>(capacity: 5)
263266
{
264-
{ HandshakeMessagePropertyNames.PID, Process.GetCurrentProcess().Id.ToString() },
267+
{ HandshakeMessagePropertyNames.PID, Environment.ProcessId.ToString(CultureInfo.InvariantCulture) },
265268
{ HandshakeMessagePropertyNames.Architecture, RuntimeInformation.ProcessArchitecture.ToString() },
266269
{ HandshakeMessagePropertyNames.Framework, RuntimeInformation.FrameworkDescription },
267270
{ HandshakeMessagePropertyNames.OS, RuntimeInformation.OSDescription },
@@ -275,11 +278,11 @@ private async Task<int> StartProcess(ProcessStartInfo processStartInfo)
275278
Logger.LogTrace(() => $"Test application arguments: {processStartInfo.Arguments}");
276279
}
277280

278-
var process = Process.Start(processStartInfo);
281+
using var process = Process.Start(processStartInfo);
279282
StoreOutputAndErrorData(process);
280283
await process.WaitForExitAsync();
281284

282-
TestProcessExited?.Invoke(this, new TestProcessExitEventArgs { OutputData = _outputData, ErrorData = _errorData, ExitCode = process.ExitCode });
285+
_handler.OnTestProcessExited(process.ExitCode, _outputData, _errorData);
283286

284287
return process.ExitCode;
285288
}
@@ -310,57 +313,33 @@ private bool ModulePathExists()
310313
{
311314
if (!File.Exists(Module.RunProperties.Command))
312315
{
313-
ErrorReceived.Invoke(this, new ErrorEventArgs { ErrorMessage = $"Test module '{Module.RunProperties.Command}' not found. Build the test application before or run 'dotnet test'." });
316+
// TODO: The error should be shown to the user, not just logged to trace.
317+
Logger.LogTrace(() => $"Test module '{Module.RunProperties.Command}' not found. Build the test application before or run 'dotnet test'.");
318+
314319
return false;
315320
}
316321
return true;
317322
}
318323

319324
public void OnHandshakeMessage(HandshakeMessage handshakeMessage, bool gotSupportedVersion)
320-
{
321-
HandshakeReceived?.Invoke(this, new HandshakeArgs { Handshake = new Handshake(handshakeMessage.Properties), GotSupportedVersion = gotSupportedVersion });
322-
}
325+
=> _handler.OnHandshakeReceived(handshakeMessage, gotSupportedVersion);
323326

324-
public void OnCommandLineOptionMessages(CommandLineOptionMessages commandLineOptionMessages)
327+
private void OnCommandLineOptionMessages(CommandLineOptionMessages commandLineOptionMessages)
325328
{
326329
HelpRequested?.Invoke(this, new HelpEventArgs { ModulePath = commandLineOptionMessages.ModulePath, CommandLineOptions = [.. commandLineOptionMessages.CommandLineOptionMessageList.Select(message => new CommandLineOption(message.Name, message.Description, message.IsHidden, message.IsBuiltIn))] });
327330
}
328331

329-
internal void OnDiscoveredTestMessages(DiscoveredTestMessages discoveredTestMessages)
330-
{
331-
DiscoveredTestsReceived?.Invoke(this, new DiscoveredTestEventArgs
332-
{
333-
ExecutionId = discoveredTestMessages.ExecutionId,
334-
InstanceId = discoveredTestMessages.InstanceId,
335-
DiscoveredTests = [.. discoveredTestMessages.DiscoveredMessages.Select(message => new DiscoveredTest(message.Uid, message.DisplayName))]
336-
});
337-
}
332+
private void OnDiscoveredTestMessages(DiscoveredTestMessages discoveredTestMessages)
333+
=> _handler.OnDiscoveredTestsReceived(discoveredTestMessages);
338334

339-
internal void OnTestResultMessages(TestResultMessages testResultMessage)
340-
{
341-
TestResultsReceived?.Invoke(this, new TestResultEventArgs
342-
{
343-
ExecutionId = testResultMessage.ExecutionId,
344-
InstanceId = testResultMessage.InstanceId,
345-
SuccessfulTestResults = [.. testResultMessage.SuccessfulTestMessages.Select(message => new SuccessfulTestResult(message.Uid, message.DisplayName, message.State, message.Duration, message.Reason, message.StandardOutput, message.ErrorOutput, message.SessionUid))],
346-
FailedTestResults = [.. testResultMessage.FailedTestMessages.Select(message => new FailedTestResult(message.Uid, message.DisplayName, message.State, message.Duration, message.Reason, [.. message.Exceptions.Select(e => new FlatException(e.ErrorMessage, e.ErrorType, e.StackTrace))], message.StandardOutput, message.ErrorOutput, message.SessionUid))]
347-
});
348-
}
335+
private void OnTestResultMessages(TestResultMessages testResultMessage)
336+
=> _handler.OnTestResultsReceived(testResultMessage);
349337

350338
internal void OnFileArtifactMessages(FileArtifactMessages fileArtifactMessages)
351-
{
352-
FileArtifactsReceived?.Invoke(this, new FileArtifactEventArgs
353-
{
354-
ExecutionId = fileArtifactMessages.ExecutionId,
355-
InstanceId = fileArtifactMessages.InstanceId,
356-
FileArtifacts = [.. fileArtifactMessages.FileArtifacts.Select(message => new FileArtifact(message.FullPath, message.DisplayName, message.Description, message.TestUid, message.TestDisplayName, message.SessionUid))]
357-
});
358-
}
339+
=> _handler.OnFileArtifactsReceived(fileArtifactMessages);
359340

360-
internal void OnSessionEvent(TestSessionEvent sessionEvent)
361-
{
362-
SessionEventReceived?.Invoke(this, new SessionEventArgs { SessionEvent = new TestSession(sessionEvent.SessionType, sessionEvent.SessionUid, sessionEvent.ExecutionId) });
363-
}
341+
private void OnSessionEvent(TestSessionEvent sessionEvent)
342+
=> _handler.OnSessionEventReceived(sessionEvent);
364343

365344
public override string ToString()
366345
{

0 commit comments

Comments
 (0)