Skip to content

Commit ff57e19

Browse files
authored
Fix stress pipeline: rename variables to avoid env var injection, simplify runner (#4329)
1 parent e46104d commit ff57e19

53 files changed

Lines changed: 295 additions & 285 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/instructions/ado-pipelines.instructions.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,20 @@ Test timeout — `--blame-hang-timeout 10m` (configured in `build.proj` and thre
110110
- `localFeedPath` = `$(Build.SourcesDirectory)/packages` — local NuGet feed for inter-package deps
111111
- `packagePath` = `$(Build.SourcesDirectory)/output` — NuGet pack output
112112

113+
## Variable Naming — Avoid `{COMMAND}ARGUMENTS` Names
114+
115+
The dotnet CLI (via System.CommandLine) reads environment variables named `{COMMAND}ARGUMENTS` and silently injects their content into the parsed arguments for that subcommand. Because Azure DevOps automatically exposes all pipeline variables as uppercased environment variables, a pipeline variable named `runArguments` becomes `RUNARGUMENTS`, which `dotnet run` reads and injects into the application's `args[]` — bypassing the `--` separator.
116+
117+
**Forbidden variable names** (any casing):
118+
- `runArguments` — injected into `dotnet run`
119+
- `buildArguments` — injected into `dotnet build`
120+
- `testArguments` — injected into `dotnet test`
121+
- Any name matching `{dotnet-subcommand}Arguments`
122+
123+
**Use instead**: `dotnetBuildOpts`, `dotnetRunOpts`, `stressTestArgs`, or other names that do not match the `{COMMAND}ARGUMENTS` pattern.
124+
125+
This affects ALL .NET SDK versions (8.0+). The injection is invisible in `[command]` log lines, making it extremely hard to diagnose. The only symptom is the application receiving unexpected arguments.
126+
113127
## Conventions When Editing Pipelines
114128

115129
- Always use templates for reusable logic — do not inline complex steps

eng/pipelines/stress/stress-tests-job.yml

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ parameters:
2424
# True to enable debugging steps.
2525
- name: debug
2626
type: boolean
27-
default: false
2827

2928
# The prefix to prepend to the job's display name:
3029
#
@@ -36,7 +35,6 @@ parameters:
3635
# The verbosity level for the dotnet CLI commands.
3736
- name: dotnetVerbosity
3837
type: string
39-
default: normal
4038
values:
4139
- quiet
4240
- minimal
@@ -48,20 +46,18 @@ parameters:
4846
- name: jobNameSuffix
4947
type: string
5048

51-
# True to fail the job when stress tests fail. When false, test failures produce warnings
52-
# (SucceededWithIssues) but do not fail the job.
53-
- name: failOnTestFailure
49+
# When true, test failures produce warnings (SucceededWithIssues) but do not fail the job.
50+
# When false, test failures fail the job. All test steps always run regardless of this setting.
51+
- name: warnOnTestFailure
5452
type: boolean
5553

5654
# The list of .NET Framework runtimes to test against.
5755
- name: netFrameworkTestRuntimes
5856
type: object
59-
default: []
6057

6158
# The list of .NET runtimes to test against.
6259
- name: netTestRuntimes
6360
type: object
64-
default: []
6561

6662
# The name of the Azure Pipelines pool to use.
6763
- name: poolName
@@ -105,18 +101,6 @@ jobs:
105101
- name: project
106102
value: $(Build.SourcesDirectory)/src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Runner/SqlClient.Stress.Runner.csproj
107103

108-
# dotnet CLI arguments for build.
109-
- name: buildArguments
110-
value: >-
111-
--verbosity ${{ parameters.dotnetVerbosity }}
112-
-p:Configuration=${{ parameters.buildConfiguration }}
113-
114-
# dotnet CLI arguments for run.
115-
- name: runArguments
116-
value: >-
117-
--verbosity ${{ parameters.dotnetVerbosity }}
118-
--configuration ${{ parameters.buildConfiguration }}
119-
120104
# The contents of the config file to use for all tests. We will write this to a JSON file and
121105
# then point to it via the STRESS_CONFIG_FILE environment variable.
122106
- name: configContent
@@ -137,9 +121,29 @@ jobs:
137121
}
138122
]
139123
140-
# Stress test command-line arguments.
141-
- name: testArguments
142-
value: -a SqlClient.Stress.Tests -console
124+
# IMPORTANT: Do NOT name pipeline variables "runArguments", "buildArguments", or
125+
# "testArguments". ADO exposes all pipeline variables as environment variables (uppercased),
126+
# and the dotnet CLI's System.CommandLine reads env vars matching {COMMAND}ARGUMENTS (e.g.
127+
# RUNARGUMENTS, BUILDARGUMENTS) and silently injects their content into the parsed arguments —
128+
# bypassing the "--" separator. This causes app arguments to contain SDK options and triggers
129+
# unintended behavior.
130+
131+
# dotnet CLI options for build.
132+
- name: dotnetBuildOpts
133+
value: >-
134+
--verbosity ${{ parameters.dotnetVerbosity }}
135+
-p:Configuration=${{ parameters.buildConfiguration }}
136+
137+
# dotnet run options shared by all test steps (framework is appended per-step).
138+
- name: dotnetRunOpts
139+
value: >-
140+
--no-build
141+
--verbosity ${{ parameters.dotnetVerbosity }}
142+
--configuration ${{ parameters.buildConfiguration }}
143+
144+
# Stress test options passed after the "--" separator.
145+
- name: stressTestOpts
146+
value: --assembly SqlClient.Stress.Tests --console
143147

144148
steps:
145149

@@ -178,7 +182,7 @@ jobs:
178182
inputs:
179183
command: build
180184
projects: $(project)
181-
arguments: $(buildArguments)
185+
arguments: ${{ variables.dotnetBuildOpts }}
182186

183187
# Set a flag so test steps can distinguish a build failure from a test failure.
184188
- pwsh: Write-Host "##vso[task.setvariable variable=buildSucceeded]true"
@@ -194,29 +198,33 @@ jobs:
194198
# - eq(variables['buildSucceeded'], 'true') gates on the flag set above, so tests are
195199
# skipped entirely if the build or any setup step failed (since there's nothing to run).
196200
#
197-
# continueOnError: ${{ not(parameters.failOnTestFailure) }}
198-
# - When failOnTestFailure is false, continueOnError is true: a test failure marks the
201+
# continueOnError: ${{ parameters.warnOnTestFailure }}
202+
# - When warnOnTestFailure is true, continueOnError is true: a test failure marks the
199203
# step and job as SucceededWithIssues (orange warning) rather than Failed.
200-
# - When failOnTestFailure is true, continueOnError is false: a test failure fails the
204+
# - When warnOnTestFailure is false, continueOnError is false: a test failure fails the
201205
# job (red), though subsequent runtimes still run due to the condition above.
202206
#
203207
- ${{ each runtime in parameters.netTestRuntimes }}:
204208
- task: DotNetCoreCLI@2
205209
displayName: Test [${{ runtime }}]
206210
condition: and(succeededOrFailed(), eq(variables['buildSucceeded'], 'true'))
207-
continueOnError: ${{ not(parameters.failOnTestFailure) }}
211+
continueOnError: ${{ parameters.warnOnTestFailure }}
212+
env:
213+
STRESS_CONFIG_FILE: config.json
208214
inputs:
209215
command: run
210216
projects: $(project)
211-
arguments: $(runArguments) --no-build -f ${{ runtime }} -e STRESS_CONFIG_FILE=config.jsonc -- $(testArguments)
217+
arguments: ${{ variables.dotnetRunOpts }} -f ${{ runtime }} -- ${{ variables.stressTestOpts }}
212218

213219
# Run the stress tests for each .NET Framework runtime.
214220
- ${{ each runtime in parameters.netFrameworkTestRuntimes }}:
215221
- task: DotNetCoreCLI@2
216222
displayName: Test [${{ runtime }}]
217223
condition: and(succeededOrFailed(), eq(variables['buildSucceeded'], 'true'))
218-
continueOnError: ${{ not(parameters.failOnTestFailure) }}
224+
continueOnError: ${{ parameters.warnOnTestFailure }}
225+
env:
226+
STRESS_CONFIG_FILE: config.json
219227
inputs:
220228
command: run
221229
projects: $(project)
222-
arguments: $(runArguments) --no-build -f ${{ runtime }} -e STRESS_CONFIG_FILE=config.jsonc -- $(testArguments)
230+
arguments: ${{ variables.dotnetRunOpts }} -f ${{ runtime }} -- ${{ variables.stressTestOpts }}

eng/pipelines/stress/stress-tests-pipeline.yml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
# ADO.Net project:
1515
# Triggering pipeline: MDS Main CI (branch internal/main only)
1616
# Pipeline name: sqlclient-stress
17-
# Pipeline URL: TODO: add URL when pipeline is created
17+
# Pipeline URL: https://dev.azure.com/SqlClientDrivers/ADO.Net/_build?definitionId=2284
1818

1919
# Set the pipeline run name to the day-of-year and the daily run counter.
2020
name: $(DayOfYear)$(Rev:rr)
@@ -73,10 +73,10 @@ parameters:
7373
type: boolean
7474
default: false
7575

76-
# True to fail the pipeline when stress tests fail. When false (default), test failures produce
77-
# warnings but do not fail the overall pipeline run.
78-
- name: failOnTestFailure
79-
displayName: Fail pipeline on test failure
76+
# When true, test failures produce warnings (SucceededWithIssues) but do not fail the pipeline.
77+
# When false (default), test failures fail the pipeline.
78+
- name: warnOnTestFailure
79+
displayName: Warn (not fail) on test failure
8080
type: boolean
8181
default: false
8282

@@ -105,5 +105,5 @@ stages:
105105
parameters:
106106
buildConfiguration: ${{ parameters.buildConfiguration }}
107107
debug: ${{ parameters.debug }}
108-
failOnTestFailure: ${{ parameters.failOnTestFailure }}
108+
warnOnTestFailure: ${{ parameters.warnOnTestFailure }}
109109
dotnetVerbosity: ${{ parameters.dotnetVerbosity }}

eng/pipelines/stress/stress-tests-stage.yml

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,15 @@ parameters:
3030
# True to enable debugging steps.
3131
- name: debug
3232
type: boolean
33-
default: false
3433

35-
# True to fail the job when stress tests fail. When false, test failures produce warnings.
36-
- name: failOnTestFailure
34+
# When true, test failures produce warnings (SucceededWithIssues) but do not fail the job.
35+
# When false, test failures fail the job. All test steps always run regardless of this setting.
36+
- name: warnOnTestFailure
3737
type: boolean
3838

3939
# The verbosity level for the dotnet CLI commands.
4040
- name: dotnetVerbosity
4141
type: string
42-
default: normal
4342
values:
4443
- quiet
4544
- minimal
@@ -52,11 +51,11 @@ parameters:
5251
type: object
5352
default: [net462]
5453

55-
# The list of .NET runtimes to test against. These must align with the TargetFrameworks defined
56-
# in the stress test Directory.Build.props and the TFMs that SqlClient ships.
54+
# The list of .NET runtimes to test against. These should include the TFMs that SqlClient ships
55+
# as well as any upcoming runtimes being validated (e.g. net10.0 is tested but not yet shipped).
5756
- name: netTestRuntimes
5857
type: object
59-
default: [net8.0, net9.0]
58+
default: [net8.0, net9.0, net10.0]
6059

6160
stages:
6261
- stage: stress_tests_stage
@@ -69,6 +68,15 @@ stages:
6968
- name: saPassword
7069
value: $[stageDependencies.secrets_stage.secrets_job.outputs['SaPassword.Value']]
7170

71+
# The 1ES pool name, determined automatically by ADO project:
72+
# - ADO.Net -> ADO-1ES-Pool
73+
# - public -> ADO-CI-1ES-Pool
74+
- name: poolName
75+
${{ if eq(variables['System.TeamProject'], 'ADO.Net') }}:
76+
value: ADO-1ES-Pool
77+
${{ else }}:
78+
value: ADO-CI-1ES-Pool
79+
7280
jobs:
7381

7482
# ----------------------------------------------------------------------------------------------
@@ -80,10 +88,12 @@ stages:
8088
debug: ${{ parameters.debug }}
8189
displayNamePrefix: Linux
8290
dotnetVerbosity: ${{ parameters.dotnetVerbosity }}
83-
failOnTestFailure: ${{ parameters.failOnTestFailure }}
91+
warnOnTestFailure: ${{ parameters.warnOnTestFailure }}
8492
jobNameSuffix: linux
93+
# No .NET Framework runtimes on Linux.
94+
netFrameworkTestRuntimes: []
8595
netTestRuntimes: ${{ parameters.netTestRuntimes }}
86-
poolName: ADO-CI-1ES-Pool
96+
poolName: ${{ variables.poolName }}
8797
saPassword: $(saPassword)
8898
sqlSetupStep:
8999
template: /eng/pipelines/common/templates/steps/configure-sql-server-linux-step.yml@self
@@ -100,12 +110,12 @@ stages:
100110
debug: ${{ parameters.debug }}
101111
displayNamePrefix: Win
102112
dotnetVerbosity: ${{ parameters.dotnetVerbosity }}
103-
failOnTestFailure: ${{ parameters.failOnTestFailure }}
113+
warnOnTestFailure: ${{ parameters.warnOnTestFailure }}
104114
jobNameSuffix: windows
105115
# Note that we include the .NET Framework runtimes for test runs on Windows.
106116
netFrameworkTestRuntimes: ${{ parameters.netFrameworkTestRuntimes }}
107117
netTestRuntimes: ${{ parameters.netTestRuntimes }}
108-
poolName: ADO-CI-1ES-Pool
118+
poolName: ${{ variables.poolName }}
109119
saPassword: $(saPassword)
110120
sqlSetupStep:
111121
template: /eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml@self
@@ -124,8 +134,10 @@ stages:
124134
debug: ${{ parameters.debug }}
125135
displayNamePrefix: macOS
126136
dotnetVerbosity: ${{ parameters.dotnetVerbosity }}
127-
failOnTestFailure: ${{ parameters.failOnTestFailure }}
137+
warnOnTestFailure: ${{ parameters.warnOnTestFailure }}
128138
jobNameSuffix: macos
139+
# No .NET Framework runtimes on macOS.
140+
netFrameworkTestRuntimes: []
129141
netTestRuntimes: ${{ parameters.netTestRuntimes }}
130142
# We don't have any 1ES Hosted Pool images for macOS, so we use a generic one from Azure
131143
# Pipelines.

src/Microsoft.Data.SqlClient/tests/StressTests/Directory.Packages.props

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,8 @@
99
<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
1010
</PropertyGroup>
1111

12+
<ItemGroup>
13+
<PackageVersion Include="System.CommandLine" Version="2.0.8" />
14+
</ItemGroup>
15+
1216
</Project>

src/Microsoft.Data.SqlClient/tests/StressTests/IMonitorLoader/IMonitorLoader.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
using System.Collections.Generic;
66

7-
namespace Monitoring
7+
namespace Microsoft.Data.SqlClient.Test.Stress
88
{
99
public interface IMonitorLoader
1010
{

src/Microsoft.Data.SqlClient/tests/StressTests/IMonitorLoader/MonitorMetrics.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
using System;
66

7-
namespace Monitoring
7+
namespace Microsoft.Data.SqlClient.Test.Stress
88
{
99
public class MonitorMetrics
1010
{

src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Common/Attributes/GlobalExceptionHandlerAttribute.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
using System;
66

7-
namespace DPStressHarness
7+
namespace Microsoft.Data.SqlClient.Test.Stress
88
{
99
[AttributeUsage(AttributeTargets.Method, Inherited = true, AllowMultiple = false)]
1010
public class GlobalExceptionHandlerAttribute : Attribute

src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Common/Attributes/GlobalTestCleanupAttribute.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
using System;
66

7-
namespace DPStressHarness
7+
namespace Microsoft.Data.SqlClient.Test.Stress
88
{
99
[AttributeUsage(AttributeTargets.Method, Inherited = true, AllowMultiple = false)]
1010
public class GlobalTestCleanupAttribute : Attribute

src/Microsoft.Data.SqlClient/tests/StressTests/SqlClient.Stress.Common/Attributes/GlobalTestSetupAttribute.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
using System;
66

7-
namespace DPStressHarness
7+
namespace Microsoft.Data.SqlClient.Test.Stress
88
{
99
[AttributeUsage(AttributeTargets.Method, Inherited = true, AllowMultiple = false)]
1010
public class GlobalTestSetupAttribute : Attribute

0 commit comments

Comments
 (0)