From ed339e75ebcb17ccce2affc417721391fcb79d97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Mon, 10 Aug 2026 19:52:37 +0200 Subject: [PATCH 1/2] Shorten data-driven test result names Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20947b29-d6e7-4513-a3eb-063051d822f0 --- .../AzureDevOpsResultPublisher.cs | 22 ++-- .../TestNameFormatter.cs | 16 ++- .../AzureDevOpsResultPublisherTests.cs | 116 ++++++++++++++++++ .../TestNameFormatterTests.cs | 22 ++++ 4 files changed, 166 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/AzureDevOpsResultPublisher.cs b/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/AzureDevOpsResultPublisher.cs index 3fbfc283b28..703678e27dd 100644 --- a/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/AzureDevOpsResultPublisher.cs +++ b/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/AzureDevOpsResultPublisher.cs @@ -305,12 +305,12 @@ static string GetResultGroupType(AggregationType aggregationType) string comment = JsonSerializer.Serialize(resultMetadata) ?? string.Empty; bool useFullyQualifiedName = _azdoParameters.UseFullyQualifiedTestName; - string DisplayNameFor(AggregatedResult result) + string DisplayNameFor(AggregatedResult result, bool isDataDrivenSubResult) => useFullyQualifiedName - ? TestNameFormatter.FormatDisplayName(result.FullyQualifiedName, result.Name) + ? TestNameFormatter.FormatDisplayName(result.FullyQualifiedName, result.Name, isDataDrivenSubResult) : result.Name; - PublishedSubResult ConvertToSubTest(AggregatedResult result) + PublishedSubResult ConvertToSubTest(AggregatedResult result, bool isDataDrivenSubResult) { var customFields = new List(); if (result.IsFlaky) @@ -327,12 +327,16 @@ PublishedSubResult ConvertToSubTest(AggregatedResult result) { Comment = comment, CustomFields = customFields, - DisplayName = DisplayNameFor(result), + DisplayName = DisplayNameFor(result, isDataDrivenSubResult), Outcome = result.Result, DurationInMs = result.DurationSeconds * 1000.0, StackTrace = result.StackTrace, ErrorMessage = result.FailureMessage, - SubResults = result.SubResults.Count == 0 ? null : [.. result.SubResults.Select(ConvertToSubTest)], + SubResults = result.SubResults.Count == 0 + ? null + : [.. result.SubResults.Select(subResult => ConvertToSubTest( + subResult, + result.AggregationType == AggregationType.DataDriven))], ResultGroupType = GetResultGroupType(result.AggregationType), }; } @@ -350,7 +354,7 @@ ConvertedResult ConvertResult(AggregatedResult result) customFields.Add(new CustomField("AttemptId", result.SubResults.Count - 1)); } - string displayName = DisplayNameFor(result); + string displayName = DisplayNameFor(result, isDataDrivenSubResult: false); return new ConvertedResult( new PublishedTestCase @@ -366,7 +370,11 @@ ConvertedResult ConvertResult(AggregatedResult result) Comment = comment, StackTrace = result.StackTrace, ErrorMessage = result.FailureMessage, - SubResults = result.SubResults.Count == 0 ? null : [.. result.SubResults.Select(ConvertToSubTest)], + SubResults = result.SubResults.Count == 0 + ? null + : [.. result.SubResults.Select(subResult => ConvertToSubTest( + subResult, + result.AggregationType == AggregationType.DataDriven))], ResultGroupType = GetResultGroupType(result.AggregationType), CustomFields = customFields, }, diff --git a/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/TestNameFormatter.cs b/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/TestNameFormatter.cs index 8e81a8d4863..d2063d0911b 100644 --- a/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/TestNameFormatter.cs +++ b/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/TestNameFormatter.cs @@ -5,8 +5,8 @@ namespace Microsoft.DotNet.Helix.AzureDevOpsTestPublisher; /// /// Builds the human-visible test title (AzDO testCaseTitle) shown when a job opts in to -/// fully qualified test names. The goal is to always surface the fully qualified name while keeping -/// any information the display name adds on top of it. +/// fully qualified test names. The goal is to surface the fully qualified name for the test while +/// keeping any information the display name adds on top of it. /// /// /// Rules (given a stable FQN = Namespace.Type.Method and a framework display name): @@ -14,13 +14,23 @@ namespace Microsoft.DotNet.Helix.AzureDevOpsTestPublisher; /// Display name is the method (the FQN's last segment) — e.g. MSTest/xUnit defaults — emit just FQN. /// Parameterized row whose base is the method — e.g. Method ("net10.0") — emit FQN ("net10.0") /// so the class prefix isn't duplicated but the argument list is preserved. +/// Rows published as children of a data-driven test keep their framework display name because +/// the parent already supplies the fully qualified context. /// Display name carries something else — e.g. a custom xUnit DisplayName — emit FQN (display name). /// /// internal static class TestNameFormatter { - public static string FormatDisplayName(string? fullyQualifiedName, string? displayName) + public static string FormatDisplayName( + string? fullyQualifiedName, + string? displayName, + bool isDataDrivenSubResult = false) { + if (isDataDrivenSubResult) + { + return displayName ?? string.Empty; + } + if (string.IsNullOrEmpty(fullyQualifiedName)) { return displayName ?? string.Empty; diff --git a/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/AzureDevOpsResultPublisherTests.cs b/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/AzureDevOpsResultPublisherTests.cs index dd50868fa81..3ee30777756 100644 --- a/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/AzureDevOpsResultPublisherTests.cs +++ b/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/AzureDevOpsResultPublisherTests.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections; +using System.Linq; using System.Net.Http; using System.Reflection; using Microsoft.DotNet.Helix.AzureDevOpsTestPublisher; @@ -29,5 +31,119 @@ public void Constructor_ConfiguresHttpClientTimeoutForLongUploads() Assert.Equal(TimeSpan.FromMinutes(5), client.Timeout); } + + [Fact] + public void FullyQualifiedNames_DataDrivenRowsUseShortChildDisplayNames() + { + const string fullyQualifiedName = + "Microsoft.DotNet.Cli.New.IntegrationTests.CommonTemplatesTests.FeaturesSupport"; + const string dataRowName = "FeaturesSupport(\"classlib\",True,\"netstandard2.0\")"; + + using var publisher = new AzureDevOpsResultPublisher( + new AzureDevOpsReportingParameters( + new Uri("https://dev.azure.com/dnceng-public/"), + "public", + "123", + "token", + UseFullyQualifiedTestName: true), + NullLogger.Instance); + + var dataRow = new AggregatedResult( + AggregationType.Single, + dataRowName, + 0.1, + "Passed", + fullyQualifiedName: fullyQualifiedName); + var test = new AggregatedResult( + AggregationType.DataDriven, + fullyQualifiedName, + 0.1, + "Passed", + [dataRow], + fullyQualifiedName: fullyQualifiedName); + + object publishedTest = ConvertSingleResult(publisher, test); + + Assert.Equal( + fullyQualifiedName, + publishedTest.GetType().GetProperty("TestCaseTitle").GetValue(publishedTest)); + + object dataRowResult = GetSingleSubResult(publishedTest); + + Assert.Equal( + dataRowName, + dataRowResult.GetType().GetProperty("DisplayName").GetValue(dataRowResult)); + } + + [Fact] + public void FullyQualifiedNames_DataDrivenRerunRowsOnlyShortenDirectChildren() + { + const string fullyQualifiedName = "Ns.MyTests.FeaturesSupport"; + const string dataRowName = "FeaturesSupport(\"classlib\")"; + + using var publisher = new AzureDevOpsResultPublisher( + new AzureDevOpsReportingParameters( + new Uri("https://dev.azure.com/dnceng-public/"), + "public", + "123", + "token", + UseFullyQualifiedTestName: true), + NullLogger.Instance); + + var attempt = new AggregatedResult( + AggregationType.Single, + $"Attempt #1 - {dataRowName}", + 0.1, + "Passed", + attemptId: 1, + fullyQualifiedName: fullyQualifiedName); + var rerunRow = new AggregatedResult( + AggregationType.Rerun, + dataRowName, + 0.1, + "Passed", + [attempt], + fullyQualifiedName: fullyQualifiedName); + var test = new AggregatedResult( + AggregationType.DataDriven, + fullyQualifiedName, + 0.1, + "Passed", + [rerunRow], + fullyQualifiedName: fullyQualifiedName); + + object publishedTest = ConvertSingleResult(publisher, test); + object publishedRow = GetSingleSubResult(publishedTest); + object publishedAttempt = GetSingleSubResult(publishedRow); + + Assert.Equal( + dataRowName, + publishedRow.GetType().GetProperty("DisplayName").GetValue(publishedRow)); + Assert.Equal( + $"{fullyQualifiedName} (Attempt #1 - {dataRowName})", + publishedAttempt.GetType().GetProperty("DisplayName").GetValue(publishedAttempt)); + } + + private static object ConvertSingleResult( + AzureDevOpsResultPublisher publisher, + AggregatedResult test) + { + MethodInfo convertResults = typeof(AzureDevOpsResultPublisher).GetMethod( + "ConvertResults", + BindingFlags.Instance | BindingFlags.NonPublic); + var convertedResults = Assert.IsAssignableFrom( + convertResults.Invoke(publisher, new object[] { new[] { test }, new object() })); + object convertedResult = Assert.Single(convertedResults.Cast()); + + return convertedResult.GetType().GetProperty("Converted").GetValue(convertedResult); + } + + private static object GetSingleSubResult(object publishedResult) + { + var subResults = Assert.IsAssignableFrom( + publishedResult.GetType().GetProperty("SubResults").GetValue(publishedResult)); + + return Assert.Single(subResults.Cast()); + } } } diff --git a/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/TestNameFormatterTests.cs b/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/TestNameFormatterTests.cs index d1151f096a7..ad986224040 100644 --- a/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/TestNameFormatterTests.cs +++ b/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/TestNameFormatterTests.cs @@ -42,6 +42,28 @@ public void ParameterizedRow_WithoutSpaceBeforeArgs_QualifiesWithoutDuplicatingM Assert.Equal("Ns.MyTests.Theory (value: 1)", result); } + [Fact] + public void ParameterizedRow_AsDataDrivenSubResult_KeepsFrameworkDisplayName() + { + string result = TestNameFormatter.FormatDisplayName( + "Microsoft.DotNet.Cli.New.IntegrationTests.CommonTemplatesTests.FeaturesSupport", + "FeaturesSupport(\"classlib\",True,\"netstandard2.0\")", + isDataDrivenSubResult: true); + + Assert.Equal("FeaturesSupport(\"classlib\",True,\"netstandard2.0\")", result); + } + + [Fact] + public void EmptyDisplayName_AsDataDrivenSubResult_DoesNotRepeatFullyQualifiedName() + { + string result = TestNameFormatter.FormatDisplayName( + "Ns.MyTests.Theory", + "", + isDataDrivenSubResult: true); + + Assert.Empty(result); + } + [Fact] public void CustomDisplayName_KeepsBothFullyQualifiedNameAndDisplayName() { From b763e20dfbf3a6315388336807129a91e5ef1c87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 11 Aug 2026 15:19:39 +0200 Subject: [PATCH 2/2] Fix job monitor timeout test race Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20947b29-d6e7-4513-a3eb-063051d822f0 --- .../JobMonitorRunnerTests.cs | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/JobMonitorRunnerTests.cs b/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/JobMonitorRunnerTests.cs index 6cea4d49d82..db75aa3b71f 100644 --- a/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/JobMonitorRunnerTests.cs +++ b/src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/JobMonitorRunnerTests.cs @@ -2113,7 +2113,14 @@ public async Task MonitorTimesOut_DoesNotReportOrCancelJobsThatFinishedAfterFirs { var azdo = new FakeAzureDevOpsService(); var helix = new FakeHelixService(); - var logger = new RecordingLogger(); + var resultsProcessed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var logger = new RecordingLogger(message => + { + if (message.Contains("test results for job 'helix-good' processed.", StringComparison.Ordinal)) + { + resultsProcessed.TrySetResult(); + } + }); azdo.AddTimelineResponse( MonitorJob(), @@ -2145,11 +2152,9 @@ public async Task MonitorTimesOut_DoesNotReportOrCancelJobsThatFinishedAfterFirs pollCount++; if (pollCount >= 2) { - // Wait until helix-good's results have actually been uploaded before - // cancelling, so the monitor has had a chance to record its terminal - // state. - Task completed = await Task.WhenAny(azdo.UploadCompleted.Task, Task.Delay(TimeSpan.FromSeconds(5))); - completed.Should().BeSameAs(azdo.UploadCompleted.Task); + // Wait until helix-good is durably marked as processed. Upload completion + // alone occurs before the monitor state is updated and races cancellation. + await resultsProcessed.Task.WaitAsync(TimeSpan.FromSeconds(5)); cts.Cancel(); } }); @@ -4008,6 +4013,13 @@ private static JobMonitorRunner CreateRunner( private sealed class RecordingLogger : ILogger { + private readonly Action _onLog; + + public RecordingLogger(Action onLog = null) + { + _onLog = onLog; + } + public List Messages { get; } = []; public IDisposable BeginScope(TState state) => NullScope.Instance; @@ -4021,7 +4033,9 @@ public void Log( Exception exception, Func formatter) { - Messages.Add(formatter(state, exception)); + string message = formatter(state, exception); + Messages.Add(message); + _onLog?.Invoke(message); } private sealed class NullScope : IDisposable