Clear the efficiency-improver backlog: OTel tags and analyzer member lookup

[Amaury Levé (Evangelink)]

Companion to #10384, which cleared the [perf-improver] backlog. This does the same for the [efficiency-improver] one tracked in #10382 / #10377.

Two of the code-level items are worth acting on; the rest are closed out as won't-fix on the issue itself, with reasons.

OpenTelemetryResultHandler.GetTestInitialInfo

Walked the property bag three times per test on the OTel path: SingleOrDefault<TestMethodIdentifierProperty>, SingleOrDefault<TestFileLocationProperty> and OfType<TestMetadataProperty>. The last one materializes a TProperty[] purely to enumerate it once.

It now uses the struct enumerator: one pass for the two singleton properties, one for the metadata. The metadata pass has to stay separate because those tags are emitted after the identifier and file-location blocks, and buffering them to fold it into a single pass would reintroduce the very allocation being removed. Two walks instead of three, and no array.

The duplicate-property detection SingleOrDefault gave for free is kept explicitly - same shape as the guards SetResultDetails already has right below - and is now covered by two tests. It is reachable: PropertyBag only rejects duplicate property instances, not two distinct instances of the same type.

DynamicDataShouldBeValidAnalyzer.TryGetMemberCore

Scanned the candidate members twice (FirstOrDefault for a property, then Where(...).ToImmutableArray() for the methods) and allocated an ImmutableArray per [DynamicData] attribute. A single switch over the members now resolves the property, the first method, and the more-than-one-method case in one pass. The backlog entry called this "compile-time only", but analyzers re-run on every keystroke in the IDE, so it is not.

Backlog items deliberately not taken

Documented on #10382 rather than changed here:

• TerminalTestReporter.TotalTests - the entry claims _assemblies.Values.Sum() runs "on every display refresh". It does not: the property has no callers in src/ at all, only in unit tests. Progress rendering and the summary compute their totals separately. Nothing to fix.
• OpenTelemetryResultHandler.GetSuiteName - merging its walk into SetResultDetails would mean reordering the metric emission relative to span tagging and buffering the metadata/artifact properties. It is a non-allocating walk over a handful of properties on an opt-in path.
• TestExecutionManager MethodLevel Select(t => new[] { t }) - the queue element type is IEnumerable<UnitTestElement>; removing the one-element array means changing that type. One-time setup allocation, not worth the churn.
• CI output-byte-count health metric - infrastructure proposal, needs a maintainer decision rather than a PR.

Validation

Microsoft.Testing.Platform.UnitTests (2024 + the 2 new tests) and MSTest.Analyzers.UnitTests (1688) all pass; full build.cmd is clean with 0 warnings.

[Copilot]

Pull request overview

Optimizes OpenTelemetry tag extraction and DynamicData analyzer member lookup without changing intended behavior.

Changes:

• Replaces repeated property-bag scans and metadata allocation with struct enumerators.
• Preserves and tests duplicate singleton-property detection.
• Resolves analyzer members in one pass without an intermediate array.

Show a summary per file

File	Description
OpenTelemetryResultHandler.cs	Optimizes property and metadata traversal.
OpenTelemetryResultHandlerTests.cs	Tests duplicate-property handling.
DynamicDataShouldBeValidAnalyzer.cs	Optimizes candidate-member lookup.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Balanced

[github-actions]

Note

🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Summary

Clean, well-motivated performance improvements — both changes eliminate redundant iterations and unnecessary allocations on hot paths.

Analyzer change (DynamicDataShouldBeValidAnalyzer): Replaces two LINQ passes (FirstOrDefault + Where().ToImmutableArray()) with a single foreach loop. Logic is preserved correctly: property wins immediately, first method is kept, multiple methods trigger the error. 👍

OTel handler change (OpenTelemetryResultHandler): Replaces two SingleOrDefault<T>() calls and one OfType<T>() enumeration with struct-enumerator passes that allocate nothing. The duplicate-property guards are a nice defensive addition (with tests).

Tests: New tests cover the duplicate-property throwing behavior. Refactoring of the helper to accept PropertyBag directly is clean.

No correctness, thread-safety, or public API surface concerns. One minor suggestion left inline (early-exit from the first enumerator loop).

Score: 95 / 100 — Excellent.

[github-actions]

🧪 Test quality grade — PR #10397

Grade	Test	Mutation	Notes	How to improve
A (90–100)	new OpenTelemetryResultHandlerTests. NotifyInProgress_ WithDuplicateIdentifierProperty_ Throws	3/3 killed	ThrowsExactly + exact message check kills all meaningful mutations on the new guard.	—
A (90–100)	new OpenTelemetryResultHandlerTests. NotifyInProgress_ WithDuplicateFileLocationProperty_ Throws	3/3 killed	ThrowsExactly + exact message check kills all meaningful mutations on the new guard.	—

_{This advisory comment was generated automatically. Grades are heuristic and informational — they do not block merging. Suggestions on the Files changed tab can be applied with one click. Re-run with /grade-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · sonnet46 21.7 AIC · ⌖ 4.17 AIC · ⊞ 11.8K · [◷]( · ◷)