Optimize VSTestBridge property lookup

[Amaury Levé (Evangelink)]

Summary

• scan only custom TestCase properties when preserving the original executor URI
• avoid TestCase.Properties concatenation and key-snapshot allocations on the per-test-case hot path
• preserve existing behavior for repeated fixups, stored null values, and executor URI replacement
• add focused regression coverage

Performance

Independent .NET 9 measurements used Microsoft.TestPlatform.ObjectModel 18.8.0, four custom properties, five 5,000,000-call repetitions, and hit-first/hit-last/no-hit scenarios. The original non-capturing predicate was confirmed to be cached; the savings come from avoiding TestCase.Properties enumeration.

Scenario	Before	After	Before allocation	After allocation
hit-first custom	542 ns/call	55 ns/call	200 B/call	64 B/call
hit-last custom	593 ns/call	190 ns/call	200 B/call	64 B/call
no hit	585 ns/call	202 ns/call	200 B/call	64 B/call

A manual loop retained the same allocations as each corresponding LINQ path, so this keeps the simpler Any expression.

Testing

• warning-free build for net462, net8.0, and net9.0
• full Microsoft.Testing.Extensions.VSTestBridge.UnitTests suites: 69/69 (net462), 74/74 (net8.0), 74/74 (net9.0)

Closes #10585

[Copilot]

Pull request overview

Optimizes the per-test-case VSTest bridge executor URI lookup while preserving existing behavior.

Changes:

• Scans only stored custom properties.
• Adds regression coverage for repeated fixups, null values, and URI replacement.

Show a summary per file

File	Description
ObjectModelConverters.cs	Optimizes original executor URI detection.
ObjectModelConvertersTests.cs	Covers fixup behavior and edge cases.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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

[github-actions]

🧵 Parallel-safety audit — PR #10586

Parallelization — assemblies audited:

Test assembly	Scope	Workers	Analyzer coverage
Microsoft.Testing.Extensions.VSTestBridge.UnitTests	MethodLevel ([assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs, unchanged by this PR)	CPU count	coverable once the parallel-safety analyzers ship (attribute-based opt-in)

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: none.

This PR only touches ObjectModelConverters.FixUpTestCase (production) and adds four new [TestMethod]s plus one private static readonly TestProperty field to ObjectModelConvertersTests.cs. Reviewed against the taxonomy:

• Category A/B (process-global state / shared paths). None. Every new test constructs its own local TestCase/TestResult instance and reads/writes only that instance's own property bag via SetPropertyValue/GetPropertyValue/GetProperties(). No environment variables, current directory, console, culture, registry, AppContext, or shared filesystem paths are touched.
• TestProperty.Register static field. The new OriginalExecutorUriProperty field mirrors the existing ClientInfo field: it is initialized once at class-load time (not mutated per test), and TestProperty.Register for a given key/type is idempotent in VSTest's object model — re-registration under the same id is a read-then-return, not a write race. Since the assembly runs at MethodLevel, this would matter if the field were mutated per test, but it never is; not a finding.
• Category C (declaration reconciliation). No [ResourceLock] / [DoNotParallelize] exists on this class or these methods, and correctly so — none of the four new tests touch a resource that requires coordination.
• Category D (over-serialization). Not applicable; nothing here is locked or deferred.

No changes to .runsettings, testconfig.json, .csproj/.props/.targets, or the assembly-level [Parallelize] attribute were made by this PR.

Top actions: None — no changes needed for parallel-safety.

_{Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.}

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 57.9 AIC · ⌖ 11.2 AIC · ⊞ 24.8K · [◷]( · ◷)

[Youssef Fahmy (Youssef1313)]

Is this whole condition even needed? Looks like it's always true to me and the property can be set unconditionally?