Skip to content

Commit cec0a3c

Browse files
authored
Discovery indicator fixes (#50903)
1 parent 8ff64a9 commit cec0a3c

File tree

5 files changed

+120
-79
lines changed

5 files changed

+120
-79
lines changed

src/Cli/dotnet/Commands/Test/MTP/Terminal/AnsiTerminalTestProgressFrame.cs

Lines changed: 55 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -26,60 +26,78 @@ public void AppendTestWorkerProgress(TestProgressState progress, RenderedProgres
2626

2727
int nonReservedWidth = Width - (durationString.Length + 2);
2828

29+
int discovered = progress.DiscoveredTests;
2930
int passed = progress.PassedTests;
3031
int failed = progress.FailedTests;
3132
int skipped = progress.SkippedTests;
3233
int retried = progress.RetriedFailedTests;
3334
int charsTaken = 0;
3435

35-
terminal.Append('[');
36-
charsTaken++;
37-
terminal.SetColor(TerminalColor.DarkGreen);
38-
terminal.Append('✓');
39-
charsTaken++;
40-
string passedText = passed.ToString(CultureInfo.CurrentCulture);
41-
terminal.Append(passedText);
42-
charsTaken += passedText.Length;
43-
terminal.ResetColor();
36+
if (!progress.IsDiscovery)
37+
{
38+
terminal.Append('[');
39+
charsTaken++;
40+
terminal.SetColor(TerminalColor.DarkGreen);
41+
terminal.Append('✓');
42+
charsTaken++;
43+
string passedText = passed.ToString(CultureInfo.CurrentCulture);
44+
terminal.Append(passedText);
45+
charsTaken += passedText.Length;
46+
terminal.ResetColor();
4447

45-
terminal.Append('/');
46-
charsTaken++;
48+
terminal.Append('/');
49+
charsTaken++;
4750

48-
terminal.SetColor(TerminalColor.DarkRed);
49-
terminal.Append('x');
50-
charsTaken++;
51-
string failedText = failed.ToString(CultureInfo.CurrentCulture);
52-
terminal.Append(failedText);
53-
charsTaken += failedText.Length;
54-
terminal.ResetColor();
51+
terminal.SetColor(TerminalColor.DarkRed);
52+
terminal.Append('x');
53+
charsTaken++;
54+
string failedText = failed.ToString(CultureInfo.CurrentCulture);
55+
terminal.Append(failedText);
56+
charsTaken += failedText.Length;
57+
terminal.ResetColor();
5558

56-
terminal.Append('/');
57-
charsTaken++;
59+
terminal.Append('/');
60+
charsTaken++;
5861

59-
terminal.SetColor(TerminalColor.DarkYellow);
60-
terminal.Append('↓');
61-
charsTaken++;
62-
string skippedText = skipped.ToString(CultureInfo.CurrentCulture);
63-
terminal.Append(skippedText);
64-
charsTaken += skippedText.Length;
65-
terminal.ResetColor();
62+
terminal.SetColor(TerminalColor.DarkYellow);
63+
terminal.Append('↓');
64+
charsTaken++;
65+
string skippedText = skipped.ToString(CultureInfo.CurrentCulture);
66+
terminal.Append(skippedText);
67+
charsTaken += skippedText.Length;
68+
terminal.ResetColor();
69+
70+
if (retried > 0)
71+
{
72+
terminal.Append('/');
73+
charsTaken++;
74+
terminal.SetColor(TerminalColor.Gray);
75+
terminal.Append('r');
76+
charsTaken++;
77+
string retriedText = retried.ToString(CultureInfo.CurrentCulture);
78+
terminal.Append(retriedText);
79+
charsTaken += retriedText.Length;
80+
terminal.ResetColor();
81+
}
6682

67-
if (retried > 0)
83+
terminal.Append(']');
84+
charsTaken++;
85+
}
86+
else
6887
{
69-
terminal.Append('/');
88+
string discoveredText = progress.DiscoveredTests.ToString(CultureInfo.CurrentCulture);
89+
terminal.Append('[');
7090
charsTaken++;
71-
terminal.SetColor(TerminalColor.Gray);
72-
terminal.Append('r');
91+
terminal.SetColor(TerminalColor.DarkMagenta);
92+
terminal.Append('+');
7393
charsTaken++;
74-
string retriedText = retried.ToString(CultureInfo.CurrentCulture);
75-
terminal.Append(retriedText);
76-
charsTaken += retriedText.Length;
94+
terminal.Append(discoveredText);
95+
charsTaken += discoveredText.Length;
7796
terminal.ResetColor();
97+
terminal.Append(']');
98+
charsTaken++;
7899
}
79100

80-
terminal.Append(']');
81-
charsTaken++;
82-
83101
terminal.Append(' ');
84102
charsTaken++;
85103
AppendToWidth(terminal, progress.AssemblyName, nonReservedWidth, ref charsTaken);

src/Cli/dotnet/Commands/Test/MTP/Terminal/SimpleTerminalBase.cs

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -67,32 +67,49 @@ public void RenderProgress(TestProgressState?[] progress)
6767

6868
string durationString = HumanReadableDurationFormatter.Render(p.Stopwatch.Elapsed);
6969

70+
int discovered = p.DiscoveredTests;
7071
int passed = p.PassedTests;
7172
int failed = p.FailedTests;
7273
int skipped = p.SkippedTests;
7374

74-
// Use just ascii here, so we don't put too many restrictions on fonts needing to
75-
// properly show unicode, or logs being saved in particular encoding.
76-
Append('[');
77-
SetColor(TerminalColor.DarkGreen);
78-
Append('+');
79-
Append(passed.ToString(CultureInfo.CurrentCulture));
80-
ResetColor();
75+
if (!p.IsDiscovery)
76+
{
77+
// Use just ascii here, so we don't put too many restrictions on fonts needing to
78+
// properly show unicode, or logs being saved in particular encoding.
79+
Append('[');
80+
SetColor(TerminalColor.DarkGreen);
81+
Append('+');
82+
Append(passed.ToString(CultureInfo.CurrentCulture));
83+
ResetColor();
84+
85+
Append('/');
8186

82-
Append('/');
87+
SetColor(TerminalColor.DarkRed);
88+
Append('x');
89+
Append(failed.ToString(CultureInfo.CurrentCulture));
90+
ResetColor();
8391

84-
SetColor(TerminalColor.DarkRed);
85-
Append('x');
86-
Append(failed.ToString(CultureInfo.CurrentCulture));
87-
ResetColor();
92+
Append('/');
8893

89-
Append('/');
94+
SetColor(TerminalColor.DarkYellow);
95+
Append('?');
96+
Append(skipped.ToString(CultureInfo.CurrentCulture));
97+
ResetColor();
9098

91-
SetColor(TerminalColor.DarkYellow);
92-
Append('?');
93-
Append(skipped.ToString(CultureInfo.CurrentCulture));
94-
ResetColor();
95-
Append(']');
99+
Append(']');
100+
}
101+
else
102+
{
103+
// Use just ascii here, so we don't put too many restrictions on fonts needing to
104+
// properly show unicode, or logs being saved in particular encoding.
105+
Append('[');
106+
SetColor(TerminalColor.DarkMagenta);
107+
Append('+');
108+
Append(discovered.ToString(CultureInfo.CurrentCulture));
109+
ResetColor();
110+
111+
Append(']');
112+
}
96113

97114
Append(' ');
98115
Append(p.AssemblyName);

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ private TestProgressState GetOrAddAssemblyRun(string assembly, string? targetFra
139139
return _assemblies.GetOrAdd(executionId, _ =>
140140
{
141141
IStopwatch sw = CreateStopwatch();
142-
var assemblyRun = new TestProgressState(Interlocked.Increment(ref _counter), assembly, targetFramework, architecture, sw);
142+
var assemblyRun = new TestProgressState(Interlocked.Increment(ref _counter), assembly, targetFramework, architecture, sw, _isDiscovery);
143143
int slotIndex = _terminalWithProgress.AddWorker(assemblyRun);
144144
assemblyRun.SlotIndex = slotIndex;
145145

@@ -801,7 +801,7 @@ void AppendOutputWhenPresent(string description, string? output)
801801
private static void AppendAssemblySummary(TestProgressState assemblyRun, ITerminal terminal)
802802
{
803803
terminal.ResetColor();
804-
804+
805805
AppendAssemblyLinkTargetFrameworkAndArchitecture(terminal, assemblyRun.Assembly, assemblyRun.TargetFramework, assemblyRun.Architecture);
806806
terminal.Append(' ');
807807
AppendAssemblyResult(terminal, assemblyRun);
@@ -914,15 +914,15 @@ public void AppendTestDiscoverySummary(ITerminal terminal, int? exitCode)
914914
var assemblies = _assemblies.Select(asm => asm.Value).OrderBy(a => a.Assembly).Where(a => a is not null).ToList();
915915

916916
int totalTests = _assemblies.Values.Sum(a => a.TotalTests);
917-
bool runFailed = _wasCancelled;
917+
bool runFailed = _wasCancelled || totalTests < 1;
918918

919919
foreach (TestProgressState assembly in assemblies)
920920
{
921-
terminal.Append(string.Format(CultureInfo.CurrentCulture, CliCommandStrings.DiscoveredTestsInAssembly, assembly.DiscoveredTests.Count));
921+
terminal.Append(string.Format(CultureInfo.CurrentCulture, CliCommandStrings.DiscoveredTestsInAssembly, assembly.DiscoveredTestNames.Count));
922922
terminal.Append(" - ");
923923
AppendAssemblyLinkTargetFrameworkAndArchitecture(terminal, assembly.Assembly, assembly.TargetFramework, assembly.Architecture);
924924
terminal.AppendLine();
925-
foreach ((string? displayName, string? uid) in assembly.DiscoveredTests)
925+
foreach ((string? displayName, string? uid) in assembly.DiscoveredTestNames)
926926
{
927927
if (displayName is not null)
928928
{

src/Cli/dotnet/Commands/Test/MTP/Terminal/TestProgressState.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
namespace Microsoft.DotNet.Cli.Commands.Test.Terminal;
88

9-
internal sealed class TestProgressState(long id, string assembly, string? targetFramework, string? architecture, IStopwatch stopwatch)
9+
internal sealed class TestProgressState(long id, string assembly, string? targetFramework, string? architecture, IStopwatch stopwatch, bool isDiscovery)
1010
{
1111
private readonly Dictionary<string, TestNodeInfoEntry> _testUidToResults = new();
1212

@@ -24,13 +24,15 @@ internal sealed class TestProgressState(long id, string assembly, string? target
2424

2525
public IStopwatch Stopwatch { get; } = stopwatch;
2626

27+
public int DiscoveredTests { get; private set; }
28+
2729
public int FailedTests { get; private set; }
2830

2931
public int PassedTests { get; private set; }
3032

3133
public int SkippedTests { get; private set; }
3234

33-
public int TotalTests => PassedTests + SkippedTests + FailedTests;
35+
public int TotalTests => IsDiscovery ? DiscoveredTests : PassedTests + SkippedTests + FailedTests;
3436

3537
public int RetriedFailedTests { get; private set; }
3638

@@ -42,10 +44,12 @@ internal sealed class TestProgressState(long id, string assembly, string? target
4244

4345
public long Version { get; internal set; }
4446

45-
public List<(string? DisplayName, string? UID)> DiscoveredTests { get; internal set; } = [];
47+
public List<(string? DisplayName, string? UID)> DiscoveredTestNames { get; internal set; } = [];
4648

4749
public bool Success { get; internal set; }
4850

51+
public bool IsDiscovery = isDiscovery;
52+
4953
public int TryCount { get; private set; }
5054

5155
private void ReportGenericTestResult(
@@ -122,8 +126,8 @@ public void ReportFailedTest(string testNodeUid, string instanceId)
122126

123127
public void DiscoverTest(string? displayName, string? uid)
124128
{
125-
PassedTests++;
126-
DiscoveredTests.Add(new(displayName, uid));
129+
DiscoveredTests++;
130+
DiscoveredTestNames.Add(new(displayName, uid));
127131
}
128132

129133
internal void NotifyHandshake(string instanceId)

test/dotnet.Tests/CommandTests/Test/TestProgressStateTests.cs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public class TestProgressStateTests
1818
public void ReportSkippedTest_MultipleCalls_DifferentInstanceId()
1919
{
2020
var stopwatchMock = new Mock<IStopwatch>();
21-
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object);
21+
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object, isDiscovery: false);
2222
string testUid = "test1";
2323
string instanceA = "instanceA";
2424
string instanceB = "instanceB";
@@ -48,7 +48,7 @@ public void ReportSkippedTest_MultipleCalls_DifferentInstanceId()
4848
public void ReportSkippedTest_RepeatedInstanceAfterRetry_ThrowsInvalidOperationException()
4949
{
5050
var stopwatchMock = new Mock<IStopwatch>();
51-
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object);
51+
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object, isDiscovery: false);
5252
string testUid = "test1";
5353
string instanceA = "instanceA";
5454
string instanceB = "instanceB";
@@ -75,7 +75,7 @@ public void ReportSkippedTest_RepeatedInstanceAfterRetry_ThrowsInvalidOperationE
7575
public void ReportFailedTest_RepeatedCalls_IncrementsFailedTests(int callCount)
7676
{
7777
var stopwatchMock = new Mock<IStopwatch>();
78-
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object);
78+
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object, isDiscovery: false);
7979
state.NotifyHandshake("instance1");
8080
for (int i = 0; i < callCount; i++)
8181
{
@@ -95,7 +95,7 @@ public void ReportFailedTest_RepeatedCalls_IncrementsFailedTests(int callCount)
9595
public void ReportFailedTest_DifferentInstanceId_RetriesFailureAndResetsCount()
9696
{
9797
var stopwatchMock = new Mock<IStopwatch>();
98-
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object);
98+
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object, isDiscovery: false);
9999
state.NotifyHandshake("id1");
100100
state.ReportFailedTest("testUid", "id1");
101101
state.ReportFailedTest("testUid", "id1");
@@ -114,7 +114,7 @@ public void ReportFailedTest_DifferentInstanceId_RetriesFailureAndResetsCount()
114114
public void ReportFailedTest_ReusingOldInstanceId_ThrowsInvalidOperationException()
115115
{
116116
var stopwatchMock = new Mock<IStopwatch>();
117-
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object);
117+
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object, isDiscovery: false);
118118
state.NotifyHandshake("id1");
119119
state.ReportFailedTest("testUid", "id1");
120120
state.NotifyHandshake("id2");
@@ -134,7 +134,7 @@ public void ReportFailedTest_ReusingOldInstanceId_ThrowsInvalidOperationExceptio
134134
public void ReportTest_WithNewInstanceId_ClearsOldReports()
135135
{
136136
var stopwatchMock = new Mock<IStopwatch>();
137-
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object);
137+
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object, isDiscovery: false);
138138
state.NotifyHandshake("id1");
139139
state.ReportFailedTest("testUid", "id1");
140140
state.ReportFailedTest("testUid", "id1");
@@ -154,6 +154,7 @@ public void ReportTest_WithNewInstanceId_ClearsOldReports()
154154
state.SkippedTests.Should().Be(1);
155155
state.RetriedFailedTests.Should().Be(3);
156156
}
157+
157158
/// <summary>
158159
/// Tests that DiscoverTest increments PassedTests and adds the displayName and uid to DiscoveredTests.
159160
/// </summary>
@@ -173,22 +174,23 @@ public void DiscoverTest_DisplayNameAndUid_AddsEntryAndIncrementsPassedTests(str
173174
assembly: "assembly.dll",
174175
targetFramework: null,
175176
architecture: null,
176-
stopwatch: stopwatchMock.Object);
177+
stopwatch: stopwatchMock.Object,
178+
isDiscovery: true);
177179

178180
state.DiscoverTest(displayName, uid);
179181

180-
state.PassedTests.Should().Be(1);
181-
state.DiscoveredTests.Count.Should().Be(1);
182-
state.DiscoveredTests[0].DisplayName.Should().Be(displayName);
183-
state.DiscoveredTests[0].UID.Should().Be(uid);
182+
state.DiscoveredTests.Should().Be(1);
183+
state.DiscoveredTestNames.Count.Should().Be(1);
184+
state.DiscoveredTestNames[0].DisplayName.Should().Be(displayName);
185+
state.DiscoveredTestNames[0].UID.Should().Be(uid);
184186
}
185187

186188
[Fact]
187189
public void FailedTestRetryShouldShouldShowTheSameTotalCountsInEachRetry()
188190
{
189191
// Tests are retried, total test count stays 3 to give use comparable counts, no matter how many times we retry.
190192
var stopwatchMock = new Mock<IStopwatch>();
191-
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object);
193+
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object, isDiscovery: false);
192194

193195
// First run
194196
state.NotifyHandshake("run1");
@@ -227,7 +229,7 @@ public void FailedTestRetryShouldNotFailTheRunWhenSecondRunProducesLessDynamicTe
227229
{
228230
// This is special test for dynamic tests where we don't know how many tests will be produced in the second run.
229231
var stopwatchMock = new Mock<IStopwatch>();
230-
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object);
232+
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object, isDiscovery: false);
231233

232234
// First run
233235
state.NotifyHandshake("run1");
@@ -259,7 +261,7 @@ public void FailedTestRetryShouldAccountPassedTestsInRetry()
259261
{
260262
// This is special test for dynamic tests where we cannot avoid re-running even non-failing tests from dynamic tests.
261263
var stopwatchMock = new Mock<IStopwatch>();
262-
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object);
264+
var state = new TestProgressState(1, "assembly.dll", null, null, stopwatchMock.Object, isDiscovery: false);
263265

264266
// First run
265267
state.NotifyHandshake("run1");

0 commit comments

Comments
 (0)