Skip to content

Conversation

@paulmedynski
Copy link
Contributor

@paulmedynski paulmedynski commented Jan 9, 2026

Description

This is a general cleanup PR, mostly focused on using the new MDS common project:

  • Update the Azure split work to start using the MDS common project
  • Eliminate the wonky MDS AssemblyName hack introduced earlier.
  • A few fixes to the Azure project tests to get them running as closely as possible to how they were when they lived in the Functional and Manual test projects.
  • Added what I believe to be a few missing #if _WINDOWS conditions.

Testing

The normal PR/CI runs will verify things generally. I have manually inspected the tests that were moved into the Azure project.

Abstractions Package - Pipeline Changes (#3628)
Move AAD/Entra Authentication into new Azure package (#3680)
User Story 39839: Move existing MDS tests to Azure package

- Identified and moved tests specific to ActiveDirectoryAuthenticationProvider into the Azure Test project.
- Updated some MDS tests that unnecessarily used ActiveDirectoryAuthenticationProvider.
- Added tests to Abstractions Test project to ensure AAD/Entra auth fails when the Azure package isn't present.
- Various build and pipeline changes to support the test changes.
- Fixed project-based builds so we can restore MDS netcore and netfx projects at the same time.
- Added caching to the managed identity provider for the ManualTest project.
- Moved username/password Entra auth provider into its own file for sharing across tests.
- Added ADO service connection for test tasks that require access to Azure resources.
- Added workload identity federation tests.
- Solved the mystery of SNI DLLs missing for .NET Framework project-reference-based builds.
- Added ADO-CI-1ES pool jobs to test the Azure package on Linux and Windows.
- Fixed Azure package pipelines to support pools in the ADO.Net and Public projects.
- Added some new pipeline variables to handle the Workload Identity Federation test.
- Removed unnecessary package ref to Microsoft.Win32.Registry.
…ffs.

- Added .NET 10 Runtime to the Abstractions and Azure package tests.
- Added the feat/* branches to CodeQL config.
- Found a few leftover relative paths for templates and made them absolute.
…meworks, and choose better TargetOs defaults.

- Updated Azure project tests to use the MDS common project.
Copilot AI review requested due to automatic review settings January 9, 2026 17:53
@paulmedynski paulmedynski changed the base branch from feat/azure-split to dev/paul/azure-split/move-tests January 9, 2026 17:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request implements infrastructure changes to enable the Azure split work by using a common MDS project and eliminating the previous MDS AssemblyName workaround. The primary mechanism introduces conditional assembly name suffixes (.NetCore and .NetFx) for project-reference builds to resolve MSBuild conflicts when building downstream projects.

Key changes:

  • Introduces ApplyMdsAssemblyNameSuffix property to conditionally append assembly name suffixes
  • Creates new test infrastructure with UsernamePasswordProvider and ManagedIdentityProvider for AAD authentication tests
  • Refactors test organization by migrating tests from MDS to Azure Extensions package
  • Updates build targets, NuGet specs, and pipeline configurations to support the new assembly naming scheme

Reviewed changes

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

Show a summary per file
File Description
src/Directory.Build.props Adds assembly name suffix logic for project-reference builds
tools/targets/GenerateMdsPackage.targets Passes assembly name suffix parameters to NuGet pack
tools/targets/CopySniDllsForNetFxProjectReferenceBuilds.targets New target file to copy SNI DLLs for .NET Framework project-reference builds
tools/specs/Microsoft.Data.SqlClient.nuspec Updates file paths to use assembly name suffix variables
src/Microsoft.Data.SqlClient/*/Microsoft.Data.SqlClient.csproj Applies assembly name suffix based on ApplyMdsAssemblyNameSuffix property
src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/UsernamePasswordProvider.cs New provider for username/password authentication in tests
src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/ManagedIdentityProvider.cs New provider for managed identity authentication in tests
src/Microsoft.Data.SqlClient.Extensions/Azure/test/* New test files migrated from MDS ManualTests
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderInternal.cs Updates to handle dynamic assembly name resolution
eng/pipelines/**/*.yml Pipeline configuration updates for new build structure

Copilot AI review requested due to automatic review settings January 9, 2026 21:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Copilot AI review requested due to automatic review settings January 13, 2026 16:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Comments suppressed due to low confidence (3)

eng/pipelines/stages/build-azure-package-ci-stage.yml:146

  • The target framework 'net10.0' does not exist as of January 2025. .NET 10.0 is not yet released. The latest released version is .NET 9.0. Unless this is intentional preparation for a future release, consider removing net10.0 from the target frameworks.
          netRuntimes: [net8.0, net9.0, net10.0]

eng/pipelines/stages/build-azure-package-ci-stage.yml:195

  • The target framework 'net10.0' does not exist as of January 2025. .NET 10.0 is not yet released. The latest released version is .NET 9.0. Unless this is intentional preparation for a future release, consider removing net10.0 from the target frameworks.
          netRuntimes: [net8.0, net9.0, net10.0]

eng/pipelines/stages/build-azure-package-ci-stage.yml:248

  • The target framework 'net10.0' does not exist as of January 2025. .NET 10.0 is not yet released. The latest released version is .NET 9.0. Unless this is intentional preparation for a future release, consider removing net10.0 from the target frameworks.
          netRuntimes: [net8.0, net9.0, net10.0]

Base automatically changed from dev/paul/azure-split/move-tests to feat/azure-split January 15, 2026 12:53
Copilot AI review requested due to automatic review settings January 15, 2026 12:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 65 out of 66 changed files in this pull request and generated 16 comments.

- Fixed terrible merge that un-did most of my changes.
- Addressed some Copilot feedback.
Copilot AI review requested due to automatic review settings January 15, 2026 13:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Copilot AI review requested due to automatic review settings January 15, 2026 16:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

@paulmedynski paulmedynski changed the title [DRAFT] Azure Split - Use MDS Common Project Azure Split - Use MDS Common Project Jan 15, 2026
@paulmedynski paulmedynski marked this pull request as ready for review January 15, 2026 19:58
@paulmedynski paulmedynski requested a review from a team as a code owner January 15, 2026 19:58
Copilot AI review requested due to automatic review settings January 15, 2026 19:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Copy link
Contributor

@benrr101 benrr101 left a comment

Choose a reason for hiding this comment

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

I'm still treating this as a work-in-progress, so as long as it works, I'm content enough with it. There's going to be lots of conflicts between this and main.

Couple recommendations imho:

  • See if you can bring some of these more general purpose changes into main directly
  • It'd be nice-ish if we can trim down some of the unrelated changes on these PRs. Like the pool names and linux build versions and sql server setup stuff isn't related to using MDS common. I'm being pretty lenient because it's not to main, but I feel like omnibus PRs would get me yelled at if I did it :)

@paulmedynski paulmedynski merged commit 0617c8e into feat/azure-split Jan 16, 2026
276 checks passed
@paulmedynski paulmedynski deleted the dev/paul/azure-split/cleanup branch January 16, 2026 13:42
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.

4 participants