Load inbox extensions without reading the RunSettingsManager singleton#16263
Merged
nohwnd merged 1 commit intoJul 13, 2026
Merged
Conversation
TestPlatform's static constructor read RunSettingsManager.Instance to pick the adapter loading strategy when it loads the inbox extensions. That read runs once at type load, before any request's run settings are known, so it only ever saw the default settings. Load with the default strategy directly instead, so this no longer depends on the shared RunSettingsManager singleton. The load stays in the static constructor because it has to run before host provider resolution reads DefaultExtensionPaths. Per request test adapter paths are still loaded through the normal PopulateExtensions flow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes a read of the process-wide RunSettingsManager.Instance from TestPlatform’s static initialization path when loading inbox extensions, so adapter loading strategy selection during type initialization no longer depends on ambient runsettings state (important for long-lived/design-mode processes serving multiple requests).
Changes:
- Stop reading
RunSettingsManager.Instance.ActiveRunSettings.SettingsXmlinTestPlatformstatic extension loading; use default settings instead. - Add clarifying comments describing why inbox extensions are loaded once at type initialization and why request-specific adapter paths still flow per-request via
PopulateExtensions.
Comment on lines
+250
to
+254
| // The inbox extensions are loaded once, at type initialization, using the default adapter | ||
| // loading strategy. We intentionally do not read the ambient RunSettingsManager singleton | ||
| // here: this runs before any request's run settings are known (see the note above), so it | ||
| // could only ever observe the default settings anyway. Request-specific test adapter paths | ||
| // are still loaded later, per request, through the normal PopulateExtensions flow. |
nohwnd
added a commit
that referenced
this pull request
Jul 13, 2026
The active run settings were shared process-wide through the RunSettingsManager.Instance singleton. In design mode one vstest.console process serves many requests, so that shared instance leaked run settings across them and forced tests to null and reset the static to isolate cases. #16263 removed the last production reader (the TestPlatform static constructor); the only references left were composition-root defaults. The Executor convenience constructor and the ArgumentProcessorFactory fallback now build a fresh, request-scoped new RunSettingsManager() instead of reading the singleton. RunSettingsManager is internal and not in the public API, so nothing outside the assembly could read it, and deleting Instance (with its backing field, lock, and setter) is safe. The tests move to a per-class new RunSettingsManager() field, so each test method gets its own instance and the save/restore and reset hygiene goes away. The two tests that only covered the singleton getter caching are removed with it. Validation: Release build clean; vstest.console.UnitTests 631/633 (2 pre-existing skips) and Common.UnitTests 393/395 pass on net11.0 and net481; smoke Acceptance 5/5 and Library 4/4 on both TFMs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TestPlatform's static constructor loads the inbox extensions (TrxLogger, the runtime providers, the legacy MSTest v1 and C++ adapters) and picked the adapter loading strategy by readingRunSettingsManager.Instance.ActiveRunSettings. That is the shared run settings static state this cleanup is about: in design mode one process serves many requests and they all observe whatever that singleton happens to hold.The read never did what it looks like it does. The static constructor runs once, at type load, before any request's run settings are known (the existing
@haploisnote above the method already calls the ordering out as incorrect), so it only ever saw the default settings anyway. The load now uses the default strategy directly and does not touchRunSettingsManager. Request specific test adapter paths are still loaded per request through the normalPopulateExtensionsflow, so/testadapterpathand<TestAdaptersPaths>are unaffected.The load stays in the static constructor on purpose. It has to run once and early, before host provider resolution reads
DefaultExtensionPaths. Moving it into the per request path makes every run fail with "No suitable test runtime provider was found", so only the source of the run settings changed, not the timing.After this the only remaining
RunSettingsManager.Instancereferences are the composition root defaults, which sets up deleting the singleton in a follow up.Build, unit tests (Client and vstest.console) and smoke tests verified locally on net11.0 and net481.
Continues the static-state cleanup from #16200/#16205/#16208/#16228/#16243/#16245/#16257/#16258/#16261/#16262.