diff --git a/src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs b/src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs index a34ea62743..d14d6ce2bb 100644 --- a/src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs +++ b/src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs @@ -179,20 +179,36 @@ private static (ISymbol? Member, bool AreTooMany) TryGetMember(INamedTypeSymbol return (null, false); } - ISymbol? potentialProperty = potentialMembers.FirstOrDefault(m => m.Kind == SymbolKind.Property); - if (potentialProperty is not null) + // Single pass over the members: a property always wins, otherwise the first method is used and + // more than one method is an error. Doing this with FirstOrDefault + Where().ToImmutableArray() + // scanned the members twice and allocated an array per [DynamicData] attribute per compilation. + ISymbol? firstMethod = null; + bool hasMultipleMethods = false; + foreach (ISymbol member in potentialMembers) { - return (potentialProperty, false); - } - - var candidateMethods = potentialMembers.Where(m => m.Kind == SymbolKind.Method).ToImmutableArray(); - if (candidateMethods.Length > 1) - { - // If there are multiple methods with the same name, report a diagnostic. This is not a supported scenario. - return (null, true); + switch (member.Kind) + { + case SymbolKind.Property: + return (member, false); + + case SymbolKind.Method: + if (firstMethod is null) + { + firstMethod = member; + } + else + { + hasMultipleMethods = true; + } + + break; + } } - return (candidateMethods.IsEmpty ? potentialMembers[0] : candidateMethods[0], false); + // If there are multiple methods with the same name, report a diagnostic. This is not a supported scenario. + return hasMultipleMethods + ? (null, true) + : (firstMethod ?? potentialMembers[0], false); } } diff --git a/src/Platform/Microsoft.Testing.Platform/Telemetry/OpenTelemetryResultHandler.cs b/src/Platform/Microsoft.Testing.Platform/Telemetry/OpenTelemetryResultHandler.cs index ac2ebfe7af..7304aa1ca8 100644 --- a/src/Platform/Microsoft.Testing.Platform/Telemetry/OpenTelemetryResultHandler.cs +++ b/src/Platform/Microsoft.Testing.Platform/Telemetry/OpenTelemetryResultHandler.cs @@ -318,7 +318,35 @@ private static string GetRunResultStatus(int failedTests, int exitCode) } } - if (testNode.Properties.SingleOrDefault() is { } identifierProperty) + // Single pass over the property bag for the two singleton properties below: two separate + // SingleOrDefault() calls walked the whole bag once each. + TestMethodIdentifierProperty? identifierProperty = null; + TestFileLocationProperty? testLocationProperty = null; + PropertyBag.PropertyBagEnumerator enumerator = testNode.Properties.GetStructEnumerator(); + while (enumerator.MoveNext()) + { + switch (enumerator.Current) + { + case TestMethodIdentifierProperty identifier: + if (identifierProperty is not null) + { + throw new InvalidOperationException($"Found multiple properties of type '{typeof(TestMethodIdentifierProperty)}'."); + } + + identifierProperty = identifier; + break; + case TestFileLocationProperty location: + if (testLocationProperty is not null) + { + throw new InvalidOperationException($"Found multiple properties of type '{typeof(TestFileLocationProperty)}'."); + } + + testLocationProperty = location; + break; + } + } + + if (identifierProperty is not null) { // code.function.name is defined as the *fully qualified* name; there is no separate namespace // attribute (code.namespace is deprecated upstream). @@ -335,7 +363,7 @@ private static string GetRunResultStatus(int failedTests, int exitCode) } } - if (testNode.Properties.SingleOrDefault() is { } testLocationProperty) + if (testLocationProperty is not null) { yield return new(TestingPlatformSemanticConventions.Attributes.CodeFilePath, testLocationProperty.FilePath); yield return new(TestingPlatformSemanticConventions.Attributes.CodeLineNumber, testLocationProperty.LineSpan.Start.Line); @@ -348,8 +376,16 @@ private static string GetRunResultStatus(int failedTests, int exitCode) } } - foreach (TestMetadataProperty metadata in testNode.Properties.OfType()) + // The metadata is yielded after the blocks above, so it needs its own pass. OfType() + // would materialize a TProperty[] just to enumerate it once; the struct enumerator allocates nothing. + PropertyBag.PropertyBagEnumerator metadataEnumerator = testNode.Properties.GetStructEnumerator(); + while (metadataEnumerator.MoveNext()) { + if (metadataEnumerator.Current is not TestMetadataProperty metadata) + { + continue; + } + yield return new KeyValuePair($"{TestingPlatformSemanticConventions.Attributes.TestMetadataPrefix}{metadata.Key}", metadata.Value); if (_options.EmitLegacyAttributes) { diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Telemetry/OpenTelemetryResultHandlerTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Telemetry/OpenTelemetryResultHandlerTests.cs index 50062b0704..60207daf45 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Telemetry/OpenTelemetryResultHandlerTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/Telemetry/OpenTelemetryResultHandlerTests.cs @@ -721,6 +721,29 @@ public void NotifyInProgress_ForTheGlobalNamespace_DoesNotEmitALeadingDot(string } private IEnumerable>? CaptureInProgressTags(TestMethodIdentifierProperty identifierProperty) + => CaptureInProgressTags(new PropertyBag(identifierProperty)); + + [TestMethod] + public void NotifyInProgress_WithDuplicateIdentifierProperty_Throws() + { + InvalidOperationException exception = Assert.ThrowsExactly(() => CaptureInProgressTags(new PropertyBag( + new TestMethodIdentifierProperty("MyAssembly", "My.Namespace", "MyClass", "MyMethod", 0, [], "void"), + new TestMethodIdentifierProperty("MyAssembly", "My.Namespace", "MyClass", "MyOtherMethod", 0, [], "void")))); + + Assert.AreEqual($"Found multiple properties of type '{typeof(TestMethodIdentifierProperty)}'.", exception.Message); + } + + [TestMethod] + public void NotifyInProgress_WithDuplicateFileLocationProperty_Throws() + { + InvalidOperationException exception = Assert.ThrowsExactly(() => CaptureInProgressTags(new PropertyBag( + new TestFileLocationProperty("first.cs", new LinePositionSpan(new LinePosition(1, 0), new LinePosition(2, 0))), + new TestFileLocationProperty("second.cs", new LinePositionSpan(new LinePosition(3, 0), new LinePosition(4, 0)))))); + + Assert.AreEqual($"Found multiple properties of type '{typeof(TestFileLocationProperty)}'.", exception.Message); + } + + private IEnumerable>? CaptureInProgressTags(PropertyBag properties) { IEnumerable>? capturedTags = null; _otelService.Setup(s => s.StartActivity( @@ -736,7 +759,7 @@ public void NotifyInProgress_ForTheGlobalNamespace_DoesNotEmitALeadingDot(string { Uid = new TestNodeUid("fqn"), DisplayName = "Test", - Properties = new PropertyBag(identifierProperty), + Properties = properties, }, null);