Finish sqlclient-stress pipeline by paulmedynski · Pull Request #4198 · dotnet/SqlClient

paulmedynski · 2026-04-15T14:14:38Z

Description

Now that the sqlclient-stress pipeline has been defined in the Public project, we can actually see it running based on the new YAML files. This PR works through issues until the stress tests run properly.

Changes

Pipeline fixes

Fix ADO.Net pipeline resource name — Updated the referenced pipeline folder/name in stress-tests-pipeline.yml after it was moved.
Fix three stress pipeline failures (Win/Linux/macOS) — Corrected various issues preventing successful pipeline execution.
Fix variable group scoping — Removed --artifacts-path from dotnet arguments, added ADO Build Properties variable group, fixed variable group casing.
Continue running stress tests after a runtime failure — Test steps now use condition: succeededOrFailed() gated on a buildSucceeded flag so all runtimes are tested even if one fails, but tests are skipped if the build/setup fails. The overall job still reports failure.
Remove unused dotnetArtifactsDir variable from stress-tests-job.yml.

SNI native DLL transitive flow

Remove PrivateAssets=All from Microsoft.Data.SqlClient.SNI in the main SqlClient csproj so the native SNI DLL flows transitively via ProjectReference. This eliminates the need for every test project to carry its own explicit SNI PackageReference (the stress tests were missing them, causing DllNotFoundException on net462).
Remove redundant SNI PackageReference entries from UnitTests, ManualTests, and FunctionalTests.
The nuspec (which is the authoritative source for NuGet package consumers) is unaffected — it independently declares the SNI dependency.

PackageReference cleanup

Replace verbose IncludeAssets with ExcludeAssets="compile" on all SNI and xunit runner PackageReferences across the codebase. Functionally equivalent but more concise.
Remove PrivateAssets="all" from SNI refs in ref/ and notsupported/ projects, and from xunit runner refs in all test projects (no project references the test projects, so it had no effect).

Checklist

Tests added or updated
Verified pipeline runs
No breaking changes introduced

Copilot

Pull request overview

This PR updates the stress-test Azure DevOps pipeline resource configuration and adjusts Managed SNI SPN generation so named-instance connections without a protocol prefix use the SSRP-resolved port (with added unit test coverage).

Changes:

Update Managed SNI SPN postfix selection to use SSRP-resolved port for non-NP protocols when connecting to named instances (incl. Protocol.None/admin:).
Add unit tests covering SPN generation for Protocol.None, tcp:, np:, admin:, and custom ServerSPN.
Fix the stress pipeline resource source path for the ADO.Net “MDS Main CI” pipeline.

Reviewed changes

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

File	Description
`src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniProxyGetSqlServerSPNsTest.cs`	Adds regression/unit tests for SPN construction across protocols and custom SPN override.
`src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs`	Adjusts SPN postfix logic to prefer SSRP-resolved port for non-NP named-instance connections; exposes helpers for unit testing.
`eng/pipelines/stress/stress-tests-pipeline.yml`	Updates the referenced ADO.Net pipeline folder/name for the stress pipeline resource trigger.

codecov · 2026-04-15T15:46:15Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.42%. Comparing base (3af6e09) to head (757430d).
⚠️ Report is 1 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (3af6e09) and HEAD (757430d). Click for more details.

HEAD has 2 uploads less than BASE

Flag BASE (3af6e09) HEAD (757430d)

CI-SqlClient 2 0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4198      +/-   ##
==========================================
- Coverage   73.62%   64.42%   -9.21%     
==========================================
  Files         277      272       -5     
  Lines       42988    65783   +22795     
==========================================
+ Hits        31650    42379   +10729     
- Misses      11338    23404   +12066

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

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

☔ View full report in Codecov by Sentry.
📢 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.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

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

dotnetArtifactsDir is still defined but is no longer used after removing --artifacts-path $(dotnetArtifactsDir) from dotnetArguments. Either remove this variable or reintroduce its usage so the job definition stays minimal and avoids confusion about where build outputs are expected.

      # The directory where dotnet artifacts will be staged.  Not to be
      # confused with pipeline artifacts.
      - name: dotnetArtifactsDir
        value: $(Build.StagingDirectory)/dotnetArtifacts

- Import 'ADO Test Configuration Properties' variable group so the configure-sql-server-win-step template gets x64AliasRegistryPath and other SQL alias variables it expects. - Align stress test TargetFrameworks with SqlClient's shipped TFMs (net8.0;net9.0) to avoid assembly version mismatches when running net8.0 tests built with the .NET 10 SDK. - Add NuGetAuthenticate@1 step before the build so hosted pool agents (macOS) can fetch upstream packages through the ADO Artifacts feed.

- Move 'ADO Test Configuration Properties' variable group from stage to job template so it is not shadowed by the job's own variables block. - Remove --artifacts-path from dotnet CLI arguments to avoid MSB3277 warnings caused by SqlClient's custom OutputPath colliding across TFMs.

- Add 'ADO Build properties' group which provides x64AliasRegistryPath, x86AliasRegistryPath, SQLAliasName, and SQLAliasPort needed by the Windows SQL Server configuration step template. - Fix 'ADO Test Configuration Properties' -> 'ADO Test Configuration properties' (lowercase 'p') to match the actual group name in ADO.

Remove PrivateAssets=All and IncludeAssets from the Microsoft.Data.SqlClient.SNI PackageReference in the main SqlClient csproj so the native SNI DLL flows transitively through ProjectReference. This eliminates the need for every test project to carry its own explicit SNI PackageReference. Remove the now-unnecessary SNI PackageReferences from UnitTests, ManualTests, and FunctionalTests csproj files.

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

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

dotnetArtifactsDir is now unused because --artifacts-path $(dotnetArtifactsDir) was removed from dotnetArguments. Either remove dotnetArtifactsDir (and related comment) to avoid confusion, or reintroduce the argument if artifacts staging is still required by downstream steps.

      # The directory where dotnet artifacts will be staged.  Not to be
      # confused with pipeline artifacts.
      - name: dotnetArtifactsDir
        value: $(Build.StagingDirectory)/dotnetArtifacts

Add name to the build step and condition test steps with succeededOrFailed() gated on the build step's success. This ensures: - All test runtimes run even if a prior runtime fails - Test steps are skipped if the build or setup fails - Any test failure still fails the overall job

Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

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

dotnetArtifactsDir is still defined, but --artifacts-path $(dotnetArtifactsDir) was removed from dotnetArguments, so this variable is now unused. Consider either removing dotnetArtifactsDir (and the related comment) or reintroducing the argument if staging build outputs is still required for troubleshooting/artifact collection.

      # The directory where dotnet artifacts will be staged.  Not to be
      # confused with pipeline artifacts.
      - name: dotnetArtifactsDir
        value: $(Build.StagingDirectory)/dotnetArtifacts

- Replace verbose IncludeAssets with ExcludeAssets="compile" on all PackageReferences that only needed to exclude compile-time assets. - Remove PrivateAssets="all" from SNI refs (ref/, notsupported/, src/) so the native DLL flows transitively via ProjectReference. The nuspec remains the authority for NuGet package consumers. - Remove PrivateAssets="all" from xunit runner refs in test projects since no project references the test projects. - Remove unused dotnetArtifactsDir variable from stress-tests-job.yml. - Fix variable group casing (ADO Build/Test Configuration Properties).

Add a top-level failOnTestFailure parameter (default: false) threaded through pipeline, stage, and job templates. When false, test failures produce warnings (SucceededWithIssues) instead of failing the pipeline. When true, test failures fail the job. Document the interaction between condition, buildSucceeded flag, and continueOnError in the job template.

Copilot

Pull request overview

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

paulmedynski · 2026-04-16T17:30:02Z


    <PackageReference Include="Microsoft.Bcl.Cryptography" />
-    <PackageReference Include="Microsoft.Data.SqlClient.SNI"
-                      PrivateAssets="all"


The PrivateAssets=all is the root cause of our test projects failing to find the SNI DLLs when building in Project ref mode. Eureka!

So I removed them all to avoid wondering why some projects have them and others don't.

paulmedynski · 2026-04-16T17:30:38Z

-    <PackageReference Include="Microsoft.Data.SqlClient.SNI"
-                      PrivateAssets="all"
-                      IncludeAssets="runtime;build;native;contentfiles;analyzers;buildtransitive" />
+    <PackageReference Include="Microsoft.Data.SqlClient.SNI" ExcludeAssets="compile" />


It's much clearer to Exclude compile than to Include everything else.

paulmedynski · 2026-04-16T17:31:13Z


    <PackageReference Include="Azure.Identity" />
    <PackageReference Include="Microsoft.Bcl.Cryptography" />
-    <PackageReference Include="Microsoft.Data.SqlClient.SNI"


We don't need these in any of our downstream (i.e. test) projects now.

paulmedynski · 2026-04-16T17:31:30Z

    <PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" />
    <PackageReference Include="System.Security.Cryptography.Pkcs" />
    <PackageReference Include="xunit" />
-    <PackageReference Include="xunit.runner.console"


I cleaned up the xUnit references as well, for consistency.

Copilot

Pull request overview

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

mdaigle · 2026-04-28T17:15:01Z

-    <PackageReference Include="Microsoft.Data.SqlClient.SNI"
-                      PrivateAssets="all"
-                      IncludeAssets="runtime;build;native;contentfiles;analyzers;buildtransitive" />
+    <PackageReference Include="Microsoft.Data.SqlClient.SNI" ExcludeAssets="compile" />


It would be nice to see a comparison of what's included in the nuget package with and without this change to understand the exact effects.

The change to the project file should have no effect on the NuGet package because we use an explicit nuspec file that already listed SNI as a dependency.

When nuget pack runs, it uses the explicit nuspec file, not the project file metadata. So the change to remove PrivateAssets=All from the .csproj:

✅ Does affect:

How the SqlClient project itself compiles and builds (the project metadata includes SNI now).

How transitive dependencies flow to projects that directly reference the project (like the test projects in the repo).

Simplifies test project configuration by eliminating redundant SNI references.

❌ Does NOT affect:

The shipped NuGet package metadata.

What consumers see when they install Microsoft.Data.SqlClient from NuGet.

The declared dependencies in the package.

Attached package comparison report.

Summary:

Package identities match (Microsoft.Data.SqlClient 7.1.0-preview1-dev).

No dependency differences.

No non-metadata payload differences.

Differences are metadata-only:

package/services/metadata/core-properties/*.psmdcp filename differs

_rels/.rels differs accordingly

Full report

NuGet Package Diff Report Generated (UTC): 2026-04-29 11:09:09Z Package A: packages\Microsoft.Data.SqlClient.7.1.0-preview1-dev.no-sni-private-assets.nupkg Package B: packages\Microsoft.Data.SqlClient.7.1.0-preview1-dev.sni-private-assets.nupkg == Identity == A: Microsoft.Data.SqlClient 7.1.0-preview1-dev B: Microsoft.Data.SqlClient 7.1.0-preview1-dev == File Set Diff == Only in A: package\services\metadata\core-properties\84b9ed528d5c42b9bc58abad9f7350b2.psmdcp Only in B: package\services\metadata\core-properties\a3351c01288e498fa28a372e7aeb47ae.psmdcp == Dependency Diff == None == Shared File Content Diff (all) == Changed: _rels\\.rels == Shared File Content Diff (excluding NuGet metadata paths) == None == Conclusion == Payload and dependencies are equivalent; differences are metadata-only.

mdaigle · 2026-04-28T17:20:05Z

-    <!-- Target all frameworks that MDS supports. -->
-    <TargetFrameworks>net8.0;net9.0;net10.0</TargetFrameworks>
+    <!-- Target the same frameworks that MDS ships (net8.0, net9.0, plus net462 on Windows). -->
+    <TargetFrameworks>net8.0;net9.0</TargetFrameworks>


New .NET versions can change environment management in ways that might affect us (e.g. thread pooling). It feels valuable to test in all available versions.

I've added .NET 10 back. This now mirrors the other SqlClient tests, but note that none of our pipelines actually run any tests with the .NET 10 runtime yet.

Copilot AI review requested due to automatic review settings April 15, 2026 14:14

github-project-automation Bot added this to SqlClient Board Apr 15, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Apr 15, 2026

paulmedynski moved this from To triage to In progress in SqlClient Board Apr 15, 2026

paulmedynski added this to the 7.1.0-preview1 milestone Apr 15, 2026

paulmedynski added the Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. label Apr 15, 2026

Copilot started reviewing on behalf of paulmedynski April 15, 2026 14:15 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs Outdated

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

Copilot AI review requested due to automatic review settings April 15, 2026 19:09

Copilot started reviewing on behalf of paulmedynski April 15, 2026 19:10 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 16, 2026 11:47

Copilot started reviewing on behalf of paulmedynski April 16, 2026 11:47 View session

paulmedynski added 5 commits April 16, 2026 08:52

Fixed name of ADO.Net pipeline since it moved recently.

04e0f09

paulmedynski force-pushed the dev/paul/stress-pipeline branch from 4df9022 to 7bf66ef Compare April 16, 2026 11:52

Copilot AI reviewed Apr 16, 2026

View reviewed changes

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

Copilot AI review requested due to automatic review settings April 16, 2026 12:56

paulmedynski force-pushed the dev/paul/stress-pipeline branch from 9d5a7b1 to c02833c Compare April 16, 2026 12:56

Copilot started reviewing on behalf of paulmedynski April 16, 2026 12:57 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj Outdated

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

paulmedynski added 2 commits April 16, 2026 10:31

Copilot AI review requested due to automatic review settings April 16, 2026 14:16

Copilot started reviewing on behalf of paulmedynski April 16, 2026 14:16 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

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

Use pwsh instead of script for cross-OS compatibility.

7af633c

paulmedynski commented Apr 16, 2026

View reviewed changes

paulmedynski marked this pull request as ready for review April 16, 2026 17:33

paulmedynski requested a review from a team as a code owner April 16, 2026 17:33

Copilot AI review requested due to automatic review settings April 16, 2026 17:33

Copilot started reviewing on behalf of paulmedynski April 16, 2026 17:33 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

paulmedynski moved this from In progress to In review in SqlClient Board Apr 16, 2026

paulmedynski added the Hotfix Candidate 🚑 Issues/PRs that are candidate for backporting to earlier supported versions. label Apr 16, 2026

cheenamalhotra previously approved these changes Apr 21, 2026

View reviewed changes

paulmedynski removed the Hotfix Candidate 🚑 Issues/PRs that are candidate for backporting to earlier supported versions. label Apr 22, 2026

paulmedynski enabled auto-merge (squash) April 22, 2026 10:58

benrr101 modified the milestones: 7.1.0-preview1, 7.1.0-preview2 Apr 27, 2026

mdaigle requested changes Apr 28, 2026

View reviewed changes

github-project-automation Bot moved this from In review to In progress in SqlClient Board Apr 28, 2026

paulmedynski added 2 commits April 29, 2026 07:04

Merge branch 'main' into dev/paul/stress-pipeline

8dfdecd

Added .NET 10 back to the Stress Tests projects.

757430d

paulmedynski dismissed cheenamalhotra’s stale review via 757430d April 29, 2026 11:14

paulmedynski moved this from In progress to In review in SqlClient Board Apr 29, 2026

Conversation

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

Uh oh!

Description

Changes

Pipeline fixes

SNI native DLL transitive flow

PackageReference cleanup

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

paulmedynski commented Apr 15, 2026 •

edited

Loading

codecov Bot commented Apr 15, 2026 •

edited

Loading