Only label SOS tests .interpreter when they actually use the interpreter#5857
Merged
max-charlamb merged 1 commit intoMay 27, 2026
Merged
Conversation
Replaces the previous per-test `UseInterpreter` flag on `SOSRunner.TestInformation` with a configuration-level model that follows the same pattern as `PublishSingleFile` / `TestCDAC`. Key pieces: * `TestConfiguration.UseInterpreter` is a per-config bool that drives both the `.interpreter` suffix in `GetStringViewWithVersion` and the `DOTNET_Interpreter=InterpTestMethod*` env-var emission in `SOSRunner`. * `SOSTestHelpers.InterpreterConfigurations` is a new `MemberData` source that returns the placeholder `TestConfiguration.Empty` row when `SOS_TEST_INTERPRETER` is unset (so the theory enumerates one row and the test skips via `SkippableTheory`). When the env var is set, it clones each default non-single-file, non-x86 configuration with `UseInterpreter=true`. * `SOSInterpreterTests` opt in by binding to `InterpreterConfigurations` and checking `!config.UseInterpreter` for the skip message; the x86 / single-file / per-test `TestInterpreter` gates are now expressed once in `InterpreterConfigurations`. * `TestConfiguration.TestInterpreter` (and its initial-dict entry) is removed; nothing other than the new opt-in path needed it. Net effect: only the two interpreter test methods receive `.interpreter`-suffixed configs and have `DOTNET_Interpreter` set on their debuggee. All other SOS tests are unaffected by `SOS_TEST_INTERPRETER`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
81a23c5 to
b8281b8
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR narrows “interpreter mode” in the SOS unit test infrastructure so that the .interpreter display-name suffix and DOTNET_Interpreter=InterpTestMethod* are applied only to the dedicated interpreter SOS tests, rather than to all SOS tests when SOS_TEST_INTERPRETER=true.
Changes:
- Introduces a per-configuration
UseInterpreterflag and uses it to control.interpretername decoration andDOTNET_Interpreteremission. - Adds
SOSTestHelpers.InterpreterConfigurationsto gate/produce interpreter-only configuration variants and updates the twoSOSInterpreterTeststo consume it. - Removes the old global
TestInterpreterconfiguration plumbing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/tests/SOS.UnitTests/SOSRunner.cs | Switches interpreter env-var emission to depend on per-config UseInterpreter. |
| src/tests/SOS.UnitTests/SOS.cs | Adds interpreter-only MemberData source and updates interpreter tests to use it. |
| src/Microsoft.Diagnostics.TestHelpers/TestConfiguration.cs | Replaces global TestInterpreter with per-config UseInterpreter and updates display-name suffixing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+581
to
+585
| /// Returns true if this configuration runs its debuggee on the CoreCLR interpreter. | ||
| /// Set only on the dedicated interpreter-variant configurations cloned by | ||
| /// SOSTestHelpers.InterpreterConfigurations; opt-in tests consume those via their | ||
| /// own MemberData source. When set, SOSRunner launches the debuggee with | ||
| /// DOTNET_Interpreter=InterpTestMethod* |
steveisok
approved these changes
May 27, 2026
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.
Summary
Reworks interpreter-test gating so the
.interpreterdisplay suffix and theDOTNET_Interpreterdebuggee env var apply only to the twoSOSInterpreterTeststest methods. Previously, settingSOS_TEST_INTERPRETER=truedecorated every SOS test name with.interpreterand setDOTNET_Interpreteron every debuggee, even though onlySOSInterpreterTestsactually exercises the interpreter.Background
When
SOS_TEST_INTERPRETER=truewas set globally, two side effects had overly-broad scope:TestConfiguration.GetStringViewWithVersionunconditionally appended.interpreterto every config''s display name. BecauseTestConfigurationis shared across all SOS theory tests, unrelated tests (e.g.SOSScenarioTests.WebApp3) showed up in xUnit results asWebApp3(*.interpreter.11.0.0-preview.5.xxxxx.yyy), misleading triage into thinking the failures were interpreter-related.SOSRunnersetDOTNET_Interpreter=InterpTestMethod*on every debuggee it launched. For non-interpreter debuggees this is functionally a no-op (no method matches the glob), but it''s still incorrect.The recent runtime-diagnostics WebApp3 R2R-skew failures (a
MissingMethodExceptionforAsyncHelpers.CaptureContextstriggered by an unrelated runtime change) were initially mis-attributed to interpreter mode because the test was labeled.interpreter-- that triage friction motivated this cleanup.Change
Interpreter mode is now a per-
TestConfigurationvariant -- analogous toPublishSingleFile/TestCDAC-- rather than a global flag observed by every test:TestConfiguration.UseInterpreteris a new per-config bool. It drives both the.interpretersuffix inGetStringViewWithVersionand theDOTNET_Interpreter=InterpTestMethod*env-var emission inSOSRunner.CreateDump/StartDebugger.SOSTestHelpers.InterpreterConfigurationsis a newMemberDatasource. WithSOS_TEST_INTERPRETERunset it returns the placeholderTestConfiguration.Emptyrow (so the theory enumerates one row and skips viaSkippableTheory). With the env var set, it clones each default non-single-file, non-x86 configuration withUseInterpreter=true.SOSInterpreterTests.InterpreterStackTestandInterpreterStackInterleavedTestbind toInterpreterConfigurationsand check!config.UseInterpreterfor the skip message. The x86 / single-file / global-gate checks now live once inInterpreterConfigurationsinstead of being duplicated per test.TestConfiguration.TestInterpreter(and its initial-dict entry) is removed; nothing other than the new opt-in path needed it.The interpreter variant is produced in code (cloning
AllSettingsand addingUseInterpreter=true); no test config XML changes are required.Net behavior
SOS_TEST_INTERPRETER=trueSOSScenarioTests.WebApp3*.interpreter.<ver>+DOTNET_Interpreterset*.<ver>, no env varSOSInterpreterTests.InterpreterStackTest*.interpreter.<ver>*.interpreter.<ver>(only this class)SOSInterpreterTests.InterpreterStackTestNo change for
SOS_TEST_INTERPRETERunset.Note
This PR description and the code changes were authored with assistance from GitHub Copilot.
Validation
runtime-diagnostics pipeline run against this PR: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1435731