Fix stress pipeline: rename variables to avoid env var injection, simplify runner by paulmedynski · Pull Request #4329 · dotnet/SqlClient

paulmedynski · 2026-06-01T13:56:42Z

Summary

Fixes stress tests running in Help mode on CI instead of executing tests. The root cause was the dotnet CLI's System.CommandLine silently injecting pipeline variable content into application arguments.

Root Cause

The dotnet CLI reads environment variables named {COMMAND}ARGUMENTS (e.g. RUNARGUMENTS, BUILDARGUMENTS, TESTARGUMENTS) and prepends their content to the parsed arguments for that subcommand. Azure DevOps automatically exposes all pipeline variables as uppercased environment variables, so a pipeline variable named buildArguments became BUILDARGUMENTS, and testArguments became TESTARGUMENTS — causing their content to leak into the application's args[] and triggering Help mode.

This affects all .NET SDK versions (8.0+) and is invisible in [command] log lines.

You can see the pipeline executing the tests now:

They aren't passing, but that's a problem for another PR.

Changes

Pipeline Default Behaviour

Changed the default bhaviour to fail the entire pipeline if any stress test steps fail.
- This is the opposite of it's original configuration, which would pass with issues if any tests failed.
- This will cause PRs to show failing checks, but will not prevent PRs from merging (stress tests aren't required checks).

Pipeline variable rename (`stress-tests-job.yml`)

Rename buildArguments → dotnetBuildOpts
Rename testArguments → stressTestOpts
Add dotnetRunOpts variable for shared dotnet run options
Use ${{ variables.* }} compile-time expansion for all static variables
Add warning comment explaining the {COMMAND}ARGUMENTS env var footgun

Pool name auto-selection (`stress-tests-stage.yml`)

Automatically select 1ES pool based on ADO project name:
- ADO.Net → ADO-1ES-Pool
- public → ADO-CI-1ES-Pool
Uses ${{ if eq(variables['System.TeamProject'], 'ADO.Net') }} at compile time
Removes hardcoded pool names from job invocations

Stress runner simplification (`Program.cs`)

Upgrade to use System.CommandLine
Remove dead code paths (RunMode.RunOne, RunMode.Profile, XML config)
Remove PlatformAttribute (unused)
Remove version detection logic (unused)
Simplify to just RunAll and Help modes

Pipeline structure (`stress-tests-pipeline.yml`, `stress-tests-stage.yml`)

Remove unnecessary parameter defaults (require explicit values)
Thread parameters cleanly through pipeline → stage → job
Explicitly pass netFrameworkTestRuntimes: [] for Linux/macOS jobs

Instructions update (`.github/instructions/ado-pipelines.instructions.md`)

Document the {COMMAND}ARGUMENTS env var injection gotcha
List forbidden variable names and safe alternatives

Verification

Confirmed the exact CI command was triggering Help mode due to env var injection
Verified variable rename eliminates the injection (Run 15321)
Confirmed env: property correctly sets STRESS_CONFIG_FILE for the spawned process
Verified pool name auto-selection compiles correctly for both projects

… warnOnTestFailure - Move STRESS_CONFIG_FILE from dotnet run -e arg to task env: property to fix DotNetCoreCLI@2 leaking the env var value as an app argument, which hit the default switch case and set RunMode.Help. - Rename failOnTestFailure to warnOnTestFailure (default false) with inverted semantics: false = failures fail the run, true = warnings only. All test steps always execute regardless of this setting.

Copilot

Pull request overview

This PR fixes Azure DevOps stress test runs accidentally executing in “Help” mode by moving STRESS_CONFIG_FILE injection from dotnet run -e ... into the supported env: block for DotNetCoreCLI@2. It also renames the failure-handling parameter to warnOnTestFailure and aligns continueOnError semantics with that name.

Changes:

Set STRESS_CONFIG_FILE via env: on the DotNetCoreCLI@2 run tasks (instead of dotnet run -e ...) to prevent argument leakage into the runner.
Replace failOnTestFailure with warnOnTestFailure (default false) and update continueOnError accordingly.
Add --no-build to shared runArguments so dotnet run consistently uses the already-built outputs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
eng/pipelines/stress/stress-tests-stage.yml	Renames and threads the new `warnOnTestFailure` parameter into the stage’s OS-specific jobs.
eng/pipelines/stress/stress-tests-pipeline.yml	Exposes `warnOnTestFailure` as a pipeline parameter and passes it to the stress test stage.
eng/pipelines/stress/stress-tests-job.yml	Applies `warnOnTestFailure` to `continueOnError` and sets `STRESS_CONFIG_FILE` via task `env:` for each runtime run.

- Remove RunMode enum, RunVerify, ExitWithError, and Help mode - Init() returns exit code directly; Main() short-circuits on non-zero - Rename RunStress() to Run() (only remaining path) - Fix GetMdsVersion() to use correct namespace (Microsoft.Data.SqlClient.ThisAssembly) - PrintHelp() dumps received args with quoting for diagnostics

The task's 'command: run' mode puts all 'arguments' content after '--' as application args, ignoring the user-specified '--' separator. This caused dotnet-run flags (--verbosity, --configuration, --no-build, -f) to be passed to the stress runner app instead of to dotnet-run itself. Switch to 'command: custom' + 'custom: run' which passes arguments literally, giving us full control over placement.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

…gression dotnet run in SDK 10.0.300 incorrectly forwards its own options (--verbosity, --configuration, --no-build) as application arguments on the CI agent (Ubuntu 22.04). This cannot be reproduced locally with the same SDK version but manifests consistently on hosted agents. Since we build separately anyway (--no-build), switch to dotnet exec which directly invokes the built DLL without any argument parsing ambiguity.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

- Remove stale -all and -verify options from PrintHelp (no run modes remain) - Clarify netTestRuntimes comment to note net10.0 is tested but not shipped - Use CmdLine@2 for .NET Framework test loop (dotnet exec cannot run net462 exe)

The dotnet CLI's System.CommandLine reads environment variables named {COMMAND}ARGUMENTS (e.g. RUNARGUMENTS, BUILDARGUMENTS, TESTARGUMENTS) and silently injects their content into parsed arguments. Since ADO exposes all pipeline variables as uppercased env vars, variables named 'runArguments', 'buildArguments', or 'testArguments' caused the stress runner to receive SDK options in its args[], triggering Help mode. Changes: - Rename buildArguments -> dotnetBuildOpts - Rename testArguments -> stressTestOpts - Add dotnetRunOpts variable for shared dotnet run options - Revert dotnet exec back to dotnet run (the real fix is the rename) - Use ${{ variables.* }} for compile-time expansion - Document the gotcha in ado-pipelines.instructions.md

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

- Upgraded the Runner to use System.CommandLine instead of trying to parse arguments by hand.

Copilot

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Runner/Monitor/MonitorLoadUtils.cs:22

mainAssembly.GetType(classname) can return null (and in the current layout it will, since mainAssembly is the Monitoring assembly that contains IMonitorLoader, but it does not define Microsoft.Data.SqlClient.Test.Stress.MonitorLoader). The subsequent t.GetInterfaces() will throw a NullReferenceException, and enabling monitoring will fail with an unhelpful crash. Add an explicit null check with a clear error so failures are actionable (and ensure the expected IMonitorLoader implementation type exists in the target assembly).

codecov · 2026-06-03T18:46:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.64%. Comparing base (3488b36) to head (af9d69e).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4329      +/-   ##
==========================================
- Coverage   66.55%   64.64%   -1.91%     
==========================================
  Files         285      280       -5     
  Lines       43311    66160   +22849     
==========================================
+ Hits        28824    42767   +13943     
- Misses      14487    23393    +8906

Flag	Coverage Δ
CI-SqlClient	`?`
PR-SqlClient-Project	`64.64% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

paulmedynski · 2026-06-09T09:18:38Z

Reviewers (@mdaigle @cheenamalhotra @benrr101) - Please don't wait for the stress test checks to pass before reviewing - they won't, because the tests don't pass reliably and I'm not going to re-run them hoping to get a green checkmark! This PR isn't concerned with fixing whatever is wrong with the stress tests. It is only concerned with getting the pipeline running reliably.

cheenamalhotra · 2026-06-09T19:11:00Z

Reviewers (@mdaigle @cheenamalhotra @benrr101) - Please don't wait for the stress test checks to pass before reviewing - they won't, because the tests don't pass reliably and I'm not going to re-run them hoping to get a green checkmark! This PR isn't concerned with fixing whatever is wrong with the stress tests. It is only concerned with getting the pipeline running reliably.

I agree the test failures need work and investigation into why are they failing at the first place. Let's also capture that in our backlog.

benrr101

Approving for pipeline instructions file

Copilot AI review requested due to automatic review settings June 1, 2026 13:56

github-project-automation Bot added this to SqlClient Board Jun 1, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Jun 1, 2026

Copilot started reviewing on behalf of paulmedynski June 1, 2026 13:56 View session

paulmedynski added Area\Tests Issues that are targeted to tests or test projects Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. labels Jun 1, 2026

paulmedynski moved this from To triage to In progress in SqlClient Board Jun 1, 2026

paulmedynski added this to the 7.1.0-preview2 milestone Jun 1, 2026

Copilot AI reviewed Jun 1, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 1, 2026 14:36

Copilot started reviewing on behalf of paulmedynski June 1, 2026 14:36 View session

Copilot AI reviewed Jun 1, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Runner/Program.cs Outdated

Comment thread eng/pipelines/stress/stress-tests-stage.yml Outdated

Copilot AI review requested due to automatic review settings June 1, 2026 15:44

Copilot started reviewing on behalf of paulmedynski June 1, 2026 15:44 View session

paulmedynski force-pushed the dev/paul/stress branch from 1b955b5 to 09a1d25 Compare June 1, 2026 15:47

Copilot AI reviewed Jun 1, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Runner/Program.cs Outdated

Comment thread eng/pipelines/stress/stress-tests-job.yml Outdated

paulmedynski added 2 commits June 1, 2026 13:04

Address Copilot review feedback on PR #4329

77fbbd6

- Remove stale -all and -verify options from PrintHelp (no run modes remain) - Clarify netTestRuntimes comment to note net10.0 is tested but not shipped - Use CmdLine@2 for .NET Framework test loop (dotnet exec cannot run net462 exe)

Copilot AI review requested due to automatic review settings June 2, 2026 19:23

Copilot started reviewing on behalf of paulmedynski June 2, 2026 19:23 View session

Copilot AI reviewed Jun 2, 2026

View reviewed changes

paulmedynski added 3 commits June 2, 2026 17:38

Choosing pool name based on Azure DevOps project name.

d2ce122

Removed unnecessary template parameter defaults.

73d38a1

Added ADO.Net pipeline URL.

40670e7

Copilot AI review requested due to automatic review settings June 3, 2026 11:10

Copilot started reviewing on behalf of paulmedynski June 3, 2026 11:11 View session

Copilot AI reviewed Jun 3, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Runner/Program.cs Outdated

Comment thread src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Runner/Program.cs Outdated

paulmedynski mentioned this pull request Jun 3, 2026

Agents | Add Azure DevOps work item markdown formatting guide & PR feedback prompt #4253

Merged

paulmedynski changed the title ~~Fix stress tests running in Help mode; replace failOnTestFailure with warnOnTestFailure~~ Fix stress pipeline: rename variables to avoid env var injection, simplify runner Jun 3, 2026

- Gave all of the stress test C# files a proper namespace.

98f2b89

- Upgraded the Runner to use System.CommandLine instead of trying to parse arguments by hand.

paulmedynski marked this pull request as ready for review June 3, 2026 12:38

paulmedynski requested review from a team and Copilot June 3, 2026 12:38

paulmedynski requested a review from a team as a code owner June 3, 2026 12:38

paulmedynski enabled auto-merge (squash) June 3, 2026 12:38

paulmedynski moved this from In progress to In review in SqlClient Board Jun 3, 2026

Copilot started reviewing on behalf of paulmedynski June 3, 2026 12:38 View session

Copilot AI reviewed Jun 3, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Runner/Program.cs

paulmedynski commented Jun 3, 2026

View reviewed changes

paulmedynski assigned mdaigle and cheenamalhotra Jun 3, 2026

mdaigle previously approved these changes Jun 3, 2026

View reviewed changes

paulmedynski assigned benrr101 Jun 3, 2026

paulmedynski requested review from benrr101 and cheenamalhotra June 3, 2026 19:12

Merge branch 'main' into dev/paul/stress

af9d69e

paulmedynski dismissed mdaigle’s stale review via af9d69e June 8, 2026 10:43

cheenamalhotra approved these changes Jun 9, 2026

View reviewed changes

mdaigle approved these changes Jun 10, 2026

View reviewed changes

benrr101 approved these changes Jun 10, 2026

View reviewed changes

paulmedynski merged commit ff57e19 into main Jun 10, 2026
301 checks passed

paulmedynski deleted the dev/paul/stress branch June 10, 2026 19:00

github-project-automation Bot moved this from In review to Done in SqlClient Board Jun 10, 2026

Uh oh!

Conversation

paulmedynski commented Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Changes

Pipeline Default Behaviour

Pipeline variable rename (stress-tests-job.yml)

Pool name auto-selection (stress-tests-stage.yml)

Stress runner simplification (Program.cs)

Pipeline structure (stress-tests-pipeline.yml, stress-tests-stage.yml)

Instructions update (.github/instructions/ado-pipelines.instructions.md)

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

paulmedynski commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cheenamalhotra commented Jun 9, 2026

Uh oh!

benrr101 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

paulmedynski commented Jun 1, 2026 •

edited

Loading

Pipeline variable rename (`stress-tests-job.yml`)

Pool name auto-selection (`stress-tests-stage.yml`)

Stress runner simplification (`Program.cs`)

Pipeline structure (`stress-tests-pipeline.yml`, `stress-tests-stage.yml`)

Instructions update (`.github/instructions/ado-pipelines.instructions.md`)

codecov Bot commented Jun 3, 2026 •

edited

Loading

paulmedynski commented Jun 9, 2026 •

edited

Loading