[Perf Improver] perf: eliminate LINQ iterator allocations in GetTestCategories and GetTestFromMethod

[Evangelink]

This pull request optimizes the way test categories and work item IDs are collected during test discovery in the MSTest adapter. The changes focus on reducing memory allocations by replacing LINQ-based code with direct iteration, improving performance and efficiency.

Fixes #7868

[Copilot]

Pull request overview

This PR optimizes MSTest adapter discovery hot paths by removing several LINQ-based enumerations and iterators during per-method metadata collection, reducing allocations and GC pressure during test discovery.

Changes:

• Reworked ReflectHelper.GetTestCategories to iterate cached attribute arrays directly rather than using chained LINQ (Concat/SelectMany).
• Updated TypeEnumerator.GetTestFromMethod to avoid LINQ allocations for parameter-type string building and to collect WorkItemAttribute IDs without OfType() passes.

Show a summary per file

File	Description
src/Adapter/MSTestAdapter.PlatformServices/Helpers/ReflectHelper.cs	Replaces LINQ category aggregation with direct iteration over cached attribute arrays.
src/Adapter/MSTestAdapter.PlatformServices/Discovery/TypeEnumerator.cs	Avoids iterator allocations for parameter type formatting and folds WorkItem ID collection into the attribute scan loop.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Evangelink]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-04-29
Repository: microsoft/testfx

Key Findings

• No test files changed — This PR is a pure performance refactoring of GetTestCategories (ReflectHelper.cs) and GetTestFromMethod (TypeEnumerator.cs), replacing LINQ chains with direct foreach loops.
• Existing test coverage is adequate — ReflectHelperTests.cs has 7 test methods exercising GetTestCategories across method/type/assembly levels. TypeEnumeratorTests.cs covers GetTestFromMethod including GetTestFromMethodShouldSetWorkItemIds (two WorkItemAttribute instances) and GetTestFromMethodShouldSetWorkItemIdsToNullIfNotAny.
• Behavior is unchanged — The refactoring preserves identical semantics: empty arrays return [], WorkItemIds is null when absent, and BeEquivalentTo assertions in the existing WorkItemIds tests confirm order-independence is acceptable.
• No isolation or flakiness concerns — No shared mutable state introduced; no timing dependencies.

Recommendations

No changes required. The optimization is clean, well-scoped, and backed by existing test coverage.

---

Generated by Test Expert Reviewer

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Evangelink]

Summary

Workflow: Expert Code Reviewer 🧠
Date: 2026-04-29
Repository: microsoft/testfx

Key Findings

No blocking issues found. The changes are correct performance improvements.

Observations (no action needed):

• WorkItemIds ordering (TypeEnumerator.cs): The original code used OfType<WorkItemAttribute>() which scanned forward (index 0→n-1), so WorkItemIds was in declaration order. The new code collects work items inside the existing backward loop (index n-1→0), meaning the array is in reversed declaration order. This is inconsequential in practice — work item IDs are used for membership-based test filtering, and the existing test uses BeEquivalentTo (order-insensitive). Worth noting for awareness, but not a bug.

• GetTestCategories refactor: The three-pass iteration over cached Attribute[] arrays exactly mirrors the behavior of the previous GetAttributes<TestCategoryBaseAttribute> calls (which also called GetCustomAttributesCached internally). Semantics are preserved, and the hot-path LINQ iterator + delegate allocations are eliminated.

• Parameter type join: parameters.Length == 0 ? string.Empty : string.Join(",", Array.ConvertAll(...)) is semantically equivalent to the original string.Join(",", GetParameters().Select(...)) — string.Join on an empty sequence returns "", same as string.Empty. The short-circuit also avoids the Array.ConvertAll allocation for parameterless methods (the common case in MSTest).

Recommendations

None — the changes are a clean, well-scoped performance optimization consistent with the allocation-free pattern already established in GetTestPropertiesAsTraits.

---

Generated by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• c50vsblobprodcus330.vsblob.vsassets.io
  • Triggering command: /usr/bin/dotnet dotnet test test/UnitTests/MSTestAdapter.UnitTests/MSTestAdapter.UnitTests.csproj -v q --filter TypeEnumerator (dns block)
• jd4vsblobprodcus366.vsblob.vsassets.io
  • Triggering command: /usr/bin/dotnet dotnet test test/UnitTests/MSTestAdapter.UnitTests/MSTestAdapter.UnitTests.csproj -v q --filter TypeEnumerator (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)