Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 27 additions & 11 deletions src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);
}
}

Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -318,7 +318,35 @@ private static string GetRunResultStatus(int failedTests, int exitCode)
}
}

if (testNode.Properties.SingleOrDefault<TestMethodIdentifierProperty>() is { } identifierProperty)
// Single pass over the property bag for the two singleton properties below: two separate
// SingleOrDefault<T>() 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;
}
}
Comment thread
Evangelink marked this conversation as resolved.

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).
Expand All@@ -335,7 +363,7 @@ private static string GetRunResultStatus(int failedTests, int exitCode)
}
}

if (testNode.Properties.SingleOrDefault<TestFileLocationProperty>() 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);
Expand All@@ -348,8 +376,16 @@ private static string GetRunResultStatus(int failedTests, int exitCode)
}
}

foreach (TestMetadataProperty metadata in testNode.Properties.OfType<TestMetadataProperty>())
// The metadata is yielded after the blocks above, so it needs its own pass. OfType<TestMetadataProperty>()
// 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<string, object?>($"{TestingPlatformSemanticConventions.Attributes.TestMetadataPrefix}{metadata.Key}", metadata.Value);
if (_options.EmitLegacyAttributes)
{
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -721,6 +721,29 @@ public void NotifyInProgress_ForTheGlobalNamespace_DoesNotEmitALeadingDot(string
}

private IEnumerable<KeyValuePair<string, object?>>? CaptureInProgressTags(TestMethodIdentifierProperty identifierProperty)
=> CaptureInProgressTags(new PropertyBag(identifierProperty));

[TestMethod]
public void NotifyInProgress_WithDuplicateIdentifierProperty_Throws()
{
InvalidOperationException exception = Assert.ThrowsExactly<InvalidOperationException>(() => 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<InvalidOperationException>(() => 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<KeyValuePair<string, object?>>? CaptureInProgressTags(PropertyBag properties)
{
IEnumerable<KeyValuePair<string, object?>>? capturedTags = null;
_otelService.Setup(s => s.StartActivity(
Expand All@@ -736,7 +759,7 @@ public void NotifyInProgress_ForTheGlobalNamespace_DoesNotEmitALeadingDot(string
{
Uid = new TestNodeUid("fqn"),
DisplayName = "Test",
Properties = new PropertyBag(identifierProperty),
Properties = properties,
},
null);

Expand Down