Avoid double TestCase construction when filtering during discovery (#9568)#9665
Conversation
…9568) Introduce UnitTestElement.GetOrCreateHostTestCase(), a lazily-cached host TestCase distinct from the always-fresh ToTestCase(). The discovery filter and the discovery sinks now reuse the cached instance, so a matched test is converted to a VSTest TestCase exactly once instead of twice (once to evaluate the filter, once to report it to the sink). The neutral ITestElementFilter / IUnitTestElementSink interfaces are unchanged; ToTestCase() still returns a fresh instance so mutation-sensitive callers are unaffected. The cache is reset on Clone/CloneWithUpdatedSource and excluded from serialization. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…double-testcase-construction # Conflicts: # src/Adapter/MSTestAdapter.PlatformServices/Execution/TestCaseDiscoverySink.cs # src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/UnitTestElement.cs
There was a problem hiding this comment.
Pull request overview
This PR eliminates a redundant TestCase allocation that occurred for every matched test during discovery. Previously, when a test-case filter was active, each UnitTestElement was converted to a VSTest TestCase twice: once inside TestElementFilter.Matches (to evaluate the filter via VSTest's MatchTestCase) and again in the discovery sink (to report the test). It fits into the ongoing platform-agnostic PlatformServices refactor (follow-up to #9567/#9568) without perturbing the neutral ITestElementFilter/IUnitTestElementSink interfaces.
The approach adds a lazily-memoized host TestCase (GetOrCreateHostTestCase()) alongside the always-fresh ToTestCase(). The filter and both discovery sinks now share the cached instance, so a matched element is converted exactly once. The cache is reset on Clone/CloneWithUpdatedSource and marked [NonSerialized] under NETFRAMEWORK, which is important because UnitTestElement[] is marshalled across the AppDomain boundary during execution.
Changes:
- Added
UnitTestElement.GetOrCreateHostTestCase()memoizing a hostTestCase, with cache invalidation in both clone paths and[NonSerialized]under NETFRAMEWORK. - Updated
TestElementFilter.Matchesand both discovery sinks (HostDiscoverySink,TestCaseDiscoverySink) to reuse the cached instance. ToTestCase()behavior is unchanged (still fresh per call), so mutation-sensitive callers are unaffected.
Show a summary per file
| File | Description |
|---|---|
| src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/UnitTestElement.cs | Adds the _hostTestCase cache field, GetOrCreateHostTestCase(), [NonSerialized] guard, and cache reset in both clone methods. |
| src/Adapter/MSTestAdapter.PlatformServices/TestMethodFilter.cs | TestElementFilter.Matches now reuses the cached host TestCase instead of building a throwaway one. |
| src/Adapter/MSTestAdapter.PlatformServices/Services/UnitTestElementSinkExtensions.cs | HostDiscoverySink.SendTestElement forwards the cached TestCase to the VSTest sink. |
| src/Adapter/MSTestAdapter.PlatformServices/Execution/TestCaseDiscoverySink.cs | Internal execution discovery sink collects the cached TestCase. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Medium
Fixes #9568.
Problem
When a test-case filter is active, every matched
UnitTestElementwas converted to a VSTestTestCasetwice during discovery:TestElementFilter.Matches— to evaluate the filter (VSTest'sMatchTestCaserequires aTestCase), andHostDiscoverySink/TestCaseDiscoverySink) — to report the matched test.For large filtered suites this doubled
TestCaseallocations (including the Id-hashing work) for every matched test. Purely redundant — no behavior/correctness impact.Fix
Added
UnitTestElement.GetOrCreateHostTestCase()— a lazily-cached hostTestCasedistinct from the always-freshToTestCase(). The discovery filter and both discovery sinks now reuse the cached instance, so a matched element is converted to aTestCaseexactly once.ToTestCase()stays fresh on every call, so mutation-sensitive callers (e.g.UnitTestElementTests, which mutate → rebuild → assert) are unaffected.ITestElementFilter/IUnitTestElementSinkinterfaces keep theirUnitTestElement-based signatures — the in-flight platform-agnostic refactor is not perturbed.Clone/CloneWithUpdatedSource(the latter changes the source, hence the Id) and excluded from serialization via[NonSerialized]underNETFRAMEWORK.Only 4 files changed (+28 / −3).
Verification
MSTestAdapter.PlatformServices.UnitTests(net9.0): 893/893 passed (filter, sink, discoverer, andUnitTestElementId tests).MSTest.IntegrationTestsTestCaseFilteringTests: 4/4 passed — the end-to-end regression guard locking FQN + Id filtering across discovery and execution.MSTestAdapter.UnitTestsMSTestDiscovererTests: 16/16 passed.