Skip to content

Test that a writer and a reader share the injected IRunSettingsHelper#16207

Merged
nohwnd merged 1 commit into
microsoft:mainfrom
nohwnd:nohwnd-test-shared-runsettingshelper
Jul 3, 2026
Merged

Test that a writer and a reader share the injected IRunSettingsHelper#16207
nohwnd merged 1 commit into
microsoft:mainfrom
nohwnd:nohwnd-test-shared-runsettingshelper

Conversation

@nohwnd

@nohwnd nohwnd commented Jul 3, 2026

Copy link
Copy Markdown
Member

#16205 injected IRunSettingsHelper into the argument processors so the request-scoped flags it carries — IsDefaultTargetArchitecture and IsDesignMode — flow through one instance instead of the RunSettingsHelper.Instance static. The whole point of that seam is that a flag written while parsing arguments is read back later through the same instance. Nothing in the suite pinned that down, so a later change could hand the writer and the reader two different objects and design-mode propagation would break with no test turning red.

This adds one test that drives both ends against a single injected instance. PlatformArgumentExecutor is the writer — parsing --Platform flips IsDefaultTargetArchitecture to false — and TestRequestManager is the reader, which consults the flag in GetDefaultArchitecture. The run settings ask for <DefaultPlatform>ARM</DefaultPlatform>, an architecture the tests never actually run on, so the two branches of the flag resolve to different results and the assertion can only pass when the reader observes the writer's flip through the shared helper. The writer gets a throwaway CommandLineOptions so the write stays isolated to the helper under test.

It is test-only; there is no product change.

Verification

  • vstest.console.UnitTests on net481: 636 passed, 0 failed, 2 skipped (638 total) — the new test is the +1 over the prior 637 total.
  • Debug and Release builds clean, no IDE0005.

…Helper

The whole point of injecting IRunSettingsHelper is that a flag written while
parsing arguments is read back later through the same instance. Nothing in the
suite pinned that down, so a later change could hand the writer and the reader
two different objects and design-mode propagation would break with no test
turning red.

This adds one test that drives both ends against a single injected instance:
PlatformArgumentExecutor is the writer (it sets IsDefaultTargetArchitecture to
false), and TestRequestManager is the reader (GetDefaultArchitecture consults
the flag). The run settings ask for <DefaultPlatform>ARM</DefaultPlatform>, an
architecture the tests never run on, so the two branches of the flag resolve to
different results and the assertion can only pass when the reader observes the
writer's flip through the shared helper. The writer gets a throwaway
CommandLineOptions to keep the write isolated to the helper under test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 3, 2026 14:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a unit test to pin down the contract introduced in #16205: argument processors that write request-scoped runsettings flags and engine components that read them must observe the same injected IRunSettingsHelper instance (to avoid silent design-mode/architecture propagation regressions).

Changes:

  • Adds a new TestRequestManagerTests test that flips IsDefaultTargetArchitecture via PlatformArgumentExecutor and verifies TestRequestManager observes the flipped value when inferring default architecture during discovery.
  • Introduces a unit-test dependency on the processors test namespace to construct the writer side (PlatformArgumentExecutor) in the same test.

Comment on lines +2905 to +2919
// GetDefaultArchitecture honors <DefaultPlatform> only while IsDefaultTargetArchitecture is true; once the flag
// is false it returns the run configuration's TargetPlatform default instead. ARM is used as the <DefaultPlatform>
// because it is never the architecture the tests actually run on, so the two branches resolve to different values
// and the assertion below can only pass if the writer's flip was observed by the reader through the shared helper.
var payload = new DiscoveryRequestPayload()
{
Sources = new List<string>() { "AnyCPU.dll" },
RunSettings =
@"<?xml version=""1.0"" encoding=""utf-8""?>
<RunSettings>
<RunConfiguration>
<DefaultPlatform>ARM</DefaultPlatform>
</RunConfiguration>
</RunSettings>"
};
@nohwnd nohwnd merged commit 1a14384 into microsoft:main Jul 3, 2026
21 checks passed
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.

2 participants