Skip to content

Abstract VSTest execution input in PlatformServices (Phase 4 of platform-agnostic effort)#9572

Merged
Evangelink merged 4 commits into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/abstract-vstest-execution-input-platform
Jul 3, 2026
Merged

Abstract VSTest execution input in PlatformServices (Phase 4 of platform-agnostic effort)#9572
Evangelink merged 4 commits into
dev/amauryleve/vstest-decoupling-basefrom
dev/amauryleve/abstract-vstest-execution-input-platform

Conversation

@Evangelink

@Evangelink Evangelink commented Jul 3, 2026

Copy link
Copy Markdown
Member

Why

Part of the multi-PR effort to make src/Adapter/MSTestAdapter.PlatformServices platform-agnostic by removing its dependency on the VSTest object model (Microsoft.TestPlatform.ObjectModel). VSTest coupling moves up into the adapter layer (src/Adapter/MSTest.TestAdapter); the platform-services engine stops reading VSTest-specific data off the test.

This is Phase 4 (execution input): it makes the execution engine (TestExecutionManager + partials) operate on the neutral UnitTestElement model for everything that reads test metadata.

What

  • Boundary conversion. TestCaseUnitTestElement now happens at the adapter boundary (MSTestExecutor, and the unit/integration test harnesses). Each element carries:
    • a neutral, string-keyed ExecutionContextProperties bag holding the host execution-context (a.k.a. TCM) properties. TcmTestPropertiesProvider.GetTcmProperties now returns IReadOnlyDictionary<string, object?> keyed by property id — lossless because GetTestContextProperties only ever consumed TestProperty.Id.
    • an opaque, [NonSerialized] HostRecordingHandle holding the originating VSTest TestCase.
  • Neutral reads. RunTestsAsync (tests overload), ExecuteTestsAsync, ExecuteTestsInSourceAsync, ExecuteTestsWithTestRunnerAsync, and MatchTestFilter no longer read VSTest test-case properties. Filtering, parallelization grouping (element.DoNotParallelize / element.TestMethod.FullClassName), ordering (managed type/method gated by HasManagedMethodAndTypeProperties, matching the old GetManagedType()/GetManagedMethod() semantics), source resolution (UnitTestElement.WithUpdatedSource), and TCM→TestContext (the neutral bag) all run off the neutral model.
  • Internal discovery. TestCaseDiscoverySink now collects UnitTestElement directly (no TestCase round-trip).

VSTest TestCase remaining — as an opaque handle only

The engine still hands the test's TestCase to two boundary services — the result recorder (ITestResultRecorder, unchanged from Phase 5) and the deployment service (ITestDeployment, TestCase-based) — but treats it as an opaque handle: it obtains it via UnitTestElement.GetOrCreateHostTestCase() (the host's original test case when present, else a single cached materialized one) and reads nothing VSTest-specific off it. This deliberately keeps recorded results byte-for-byte identical to baseline: the unchanged recorder receives the exact same TestCase the engine passed before this change, so TRX/TCM association and any host/data-collector-injected properties are preserved. This is not "100% TestCase-free" — see remaining work.

No behavior change

Pure refactor: the executed test set, order, parallelization grouping, TCM/TRX properties, and recorded results (fields, Id, outcome, messages, attachments, timings) are preserved for both the RunTestsAsync(tests) and RunTestsAsync(sources) paths. The filter Id is filename-based (source-path-independent), so source resolution does not affect the filtered set.

Source resolution and a tracked latent bug

Correct element-based source resolution is required for the deployment path (issue #6713 — the executed assembly must load from the deployment directory). TestMethod.CloneWithUpdatedSource has a latent bug: it assigns AssemblyName/MethodInfo on this instead of the returned clone, tolerated only by the current call order. Rather than change that shared primitive here, this PR leaves the buggy method untouched for the existing ToTestCase/test-case-filter bridge callers and adds a new, correct CloneWithSource that the neutral engine uses via WithUpdatedSource. The bug is tracked by #9573 and referenced in comments next to both coexisting clone methods. Correct deployment-directory loading is verified by RunTestsForTestShouldLoadSourceFromDeploymentDirectoryIfDeployed.

Remaining work (later phases)

Neutralizing the two boundary services — so the engine no longer holds a TestCase handle at all — is deferred and is the prerequisite for Phase 7 (cutting the Microsoft.TestPlatform.ObjectModel dependency from PlatformServices):

  • The result recorder neutralization (moving the element → TestCase reconstruction, incl. the host-handle / TCM / data-collector fidelity, out of the engine) — a dedicated recorder piece.
  • Deployment (ITestDeployment.Deploy, which also flows IFrameworkHandle) — folds into the deployment/contexts phase (Phase 6).
  • The single VSTest bridges that stay in PlatformServices for now: TestResultRecorderExtensions, UnitTestElementSinkExtensions, TcmTestPropertiesProvider, TestCaseExtensions/UnitTestElement.ToTestCase.
  • Unifying the two clone methods once CloneWithUpdatedSource mutates this instead of the returned clone #9573 is addressed.

Verification

  • build.cmd -c Debug green across all TFMs (net462, net8.0, net9.0, UWP, WinUI).
  • MSTestAdapter.PlatformServices.UnitTests (897) and MSTestAdapter.UnitTests (21) green; the deployment-directory test passes explicitly. Recorded-result fidelity is covered by MSTestExecutorTests (asserts RecordStart against the original host TestCase) and the integration harness's recorded-result assertions — the recorder is unchanged and receives the same TestCase as baseline.
  • MSTest.IntegrationTests (discover→run harness) green — the only failing test seen was OutputIsNotMixedWhenTestsRunInParallel, a known [CICondition(ConditionMode.Exclude)] timing-flaky parallelism assertion unrelated to this change.

Stacked PR

Base = dev/amauryleve/vstest-decoupling-base (the accumulation base that already contains Phase 2 #9555 and Phase 3 #9566). Phase 1 + Phase 5 are already on main. This should be reviewed/merged after the earlier phases reach main.

Amaury Leveque and others added 4 commits July 3, 2026 13:07
Make the MSTestAdapter.PlatformServices execution engine operate on the neutral
UnitTestElement model end-to-end instead of the VSTest TestCase object model.

- Convert TestCase -> UnitTestElement at the adapter boundary (MSTestExecutor and
  the integration/unit test harnesses), carrying host execution-context (TCM)
  properties in a neutral string-keyed ExecutionContextProperties bag and the
  originating TestCase as an opaque, non-serialized HostRecordingHandle.
- TestExecutionManager (RunTestsAsync tests-overload, ExecuteTestsAsync,
  ExecuteTestsInSourceAsync, ExecuteTestsWithTestRunnerAsync, MatchTestFilter,
  SendTestResults) and ITestResultRecorder now flow UnitTestElement.
- The result-recording bridge records against the opaque host handle when present
  (byte-for-byte fidelity incl. TCM/host properties for normal results) and falls
  back to element.ToTestCase() for internally discovered tests.
- Fix TestMethod.CloneWithUpdatedSource to assign the source on the clone.
- Deployment stays TestCase-based; the engine materializes test cases only at that
  single boundary call (deferred to a later phase).

No behavior change: executed set, order, parallelization grouping, TCM/TRX
properties and recorded results remain identical.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Reuse a single materialized VSTest test case per internally-discovered test
  (UnitTestElement.GetOrCreateHostTestCase) so deployment, test-start and every
  reported result share one instance, restoring the historical one-test-case-per-
  discovered-test invariant and avoiding repeated Id hashing on the sources path.
- Add UnitTestElement tests for WithUpdatedSource (clone gets the new source, the
  original is left unmutated) and GetOrCreateHostTestCase (host-handle reuse and
  lazy materialization caching).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Per review direction, keep the latent TestMethod/UnitTestElement.CloneWithUpdatedSource
bug (assigns source on 'this' instead of the clone) untouched for the existing
ToTestCase/test-case-filter bridge callers, and route the neutral execution engine's
source resolution through a new, correct CloneWithSource that updates the returned
clone only. The bug is tracked by #9573 and referenced in comments next to both
coexisting clone methods.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Per review decision, keep the Phase 5 ITestResultRecorder (and its bridge)
TestCase-based and unchanged. The execution engine now threads the test's host
TestCase as an OPAQUE handle (obtained via UnitTestElement.GetOrCreateHostTestCase)
into the recorder and the deployment service, reading nothing VSTest-specific off
it. This keeps recorded results byte-for-byte identical to baseline (the unchanged
recorder receives the exact same TestCase), while filtering, parallelization
grouping, source resolution and TCM->TestContext remain neutral.

Neutralizing the recorder and deployment (removing the opaque TestCase handle) is
deferred to a later boundary-neutralization phase.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@Evangelink
Evangelink marked this pull request as ready for review July 3, 2026 11:41
@Evangelink
Evangelink merged commit 4f02b6e into dev/amauryleve/vstest-decoupling-base Jul 3, 2026
20 of 22 checks passed
@Evangelink
Evangelink deleted the dev/amauryleve/abstract-vstest-execution-input-platform branch July 3, 2026 11:41
Evangelink added a commit that referenced this pull request Jul 5, 2026
…orm-agnostic effort) (#9572)

Co-authored-by: Amaury Leveque <amauryleve@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Evangelink added a commit that referenced this pull request Jul 5, 2026
…ervices

Squashed rebase of the vstest-decoupling PlatformServices stack onto main.
Phases 1, 2 and 5 already landed on main via #9548/#9567/#9550; this commit
carries the remaining net-new work:

- Phase 3  (#9566): abstract the VSTest discovery sink (IUnitTestElementSink).
- Phase 4  (#9572): abstract VSTest execution input.
- Phase 6a (#9576): neutralize deployment input (DeploymentContext).
- Phase 6b (#9579): neutralize test result recording (ITestResultRecorder).
- Phase 6c (#9585): neutralize test message logging.
- Phase 6c2:         neutralize run-settings input in the host layer (settingsXml).
- Phase 6d-1:        move test-case filter parsing to the adapter boundary
                     (ITestElementFilterProvider / TestElementFilterProvider).
- Phase 6d-2:        remove IRunContext/IDiscoveryContext from PlatformServices.
- Phase 6e-1 (#9622): remove IFrameworkHandle from the execution engine.
- Phase 6e-2 (#9623): relocate VSTest logger/sink bridges to the adapter.
- Phase 6e-3a (#9624): neutralize the trait type on UnitTestElement.

Result: MSTestAdapter.PlatformServices no longer references the VSTest
run/discovery context or result object model; those types live only at the
MSTest.TestAdapter boundary.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Evangelink added a commit that referenced this pull request Jul 5, 2026
…ervices

Squashed rebase of the vstest-decoupling PlatformServices stack onto main.
Phases 1, 2 and 5 already landed on main via #9548/#9567/#9550; this commit
carries the remaining net-new work:

- Phase 3  (#9566): abstract the VSTest discovery sink (IUnitTestElementSink).
- Phase 4  (#9572): abstract VSTest execution input.
- Phase 6a (#9576): neutralize deployment input (DeploymentContext).
- Phase 6b (#9579): neutralize test result recording (ITestResultRecorder).
- Phase 6c (#9585): neutralize test message logging.
- Phase 6c2:         neutralize run-settings input in the host layer (settingsXml).
- Phase 6d-1:        move test-case filter parsing to the adapter boundary
                     (ITestElementFilterProvider / TestElementFilterProvider).
- Phase 6d-2:        remove IRunContext/IDiscoveryContext from PlatformServices.
- Phase 6e-1 (#9622): remove IFrameworkHandle from the execution engine.
- Phase 6e-2 (#9623): relocate VSTest logger/sink bridges to the adapter.
- Phase 6e-3a (#9624): neutralize the trait type on UnitTestElement.

Result: MSTestAdapter.PlatformServices no longer references the VSTest
run/discovery context or result object model; those types live only at the
MSTest.TestAdapter boundary.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant