diff --git a/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/AzureDevOpsResultPublisher.cs b/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/AzureDevOpsResultPublisher.cs index df9ce21216d..2d268299927 100644 --- a/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/AzureDevOpsResultPublisher.cs +++ b/src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/AzureDevOpsResultPublisher.cs @@ -233,12 +233,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) @@ -255,12 +255,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), }; } @@ -278,7 +282,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 @@ -294,7 +298,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 59d61985642..dcde1b30ba1 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 System.Threading; @@ -39,11 +41,139 @@ public void Constructor_ConfiguresHttpClientTimeoutForLongUploads() NullLogger.Instance); FieldInfo field = typeof(AzureDevOpsResultPublisher).GetField("_httpClient", BindingFlags.Instance | BindingFlags.NonPublic); + Assert.NotNull(field); + var client = Assert.IsType(field.GetValue(publisher)); 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, + GetRequiredPropertyValue(publishedTest, "TestCaseTitle")); + + object dataRowResult = GetSingleSubResult(publishedTest); + + Assert.Equal( + dataRowName, + GetRequiredPropertyValue(dataRowResult, "DisplayName")); + } + + [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, + GetRequiredPropertyValue(publishedRow, "DisplayName")); + Assert.Equal( + $"{fullyQualifiedName} (Attempt #1 - {dataRowName})", + GetRequiredPropertyValue(publishedAttempt, "DisplayName")); + } + + private static object ConvertSingleResult( + AzureDevOpsResultPublisher publisher, + AggregatedResult test) + { + MethodInfo convertResults = typeof(AzureDevOpsResultPublisher).GetMethod( + "ConvertResults", + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.NotNull(convertResults); + + var convertedResults = Assert.IsAssignableFrom( + convertResults.Invoke(publisher, new object[] { new[] { test }, new object() })); + object convertedResult = Assert.Single(convertedResults.Cast()); + + return GetRequiredPropertyValue(convertedResult, "Converted"); + } + + private static object GetSingleSubResult(object publishedResult) + { + var subResults = Assert.IsAssignableFrom( + GetRequiredPropertyValue(publishedResult, "SubResults")); + + return Assert.Single(subResults.Cast()); + } + + private static object GetRequiredPropertyValue(object instance, string propertyName) + { + PropertyInfo property = instance.GetType().GetProperty(propertyName); + Assert.NotNull(property); + + object value = property.GetValue(instance); + Assert.NotNull(value); + return value; + } + [Theory] [InlineData("Passed", true)] [InlineData("NotExecuted", true)] @@ -109,6 +239,5 @@ public void CancellationWithoutTimeoutIsNotTransient() new OperationCanceledException(), CancellationToken.None)); } - } } 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 b5d59855570..d042b6a4841 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 @@ -2189,7 +2189,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(), @@ -2221,11 +2228,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(); } }); @@ -4084,6 +4089,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; @@ -4097,7 +4109,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 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..c1509b1a55f 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,30 @@ 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); + } + + [Theory] + [InlineData("")] + [InlineData(null)] + public void MissingDisplayName_AsDataDrivenSubResult_DoesNotRepeatFullyQualifiedName(string displayName) + { + string result = TestNameFormatter.FormatDisplayName( + "Ns.MyTests.Theory", + displayName, + isDataDrivenSubResult: true); + + Assert.Empty(result); + } + [Fact] public void CustomDisplayName_KeepsBothFullyQualifiedNameAndDisplayName() {