Skip to content

Conversation

cheenamalhotra
Copy link
Member

Ports #3620 to release/6.0

…3620)

* Fix SetProvider to return immediately if user-defined provider found

* Include test

* Fix tests

* Remove unwanted changes

* Update config file name

* Rename file back to app.config

* Copy always

* Disable tests for now.

* Fix framework inclusion

* Fix test failures

* Touch ups

* Fix test (continued)
@Copilot Copilot AI review requested due to automatic review settings October 3, 2025 16:50
@cheenamalhotra cheenamalhotra added this to the 6.0.3 milestone Oct 3, 2025
Copy link
Contributor

@Copilot 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

Ports the earlier fix to ensure SetProvider does not overwrite a user-defined authentication provider and adds .NET Framework-specific test coverage for configuration-based provider overrides. Key additions include a dummy authentication provider, app.config-based registration for .NET Framework functional tests, and related test adjustments.

  • Prevents overwriting an existing user-defined provider by returning early in SetProvider.
  • Adds dummy provider + app.config to validate provider override behavior on .NET Framework.
  • Introduces new tests and conditional compilation for framework-specific behavior.

Reviewed Changes

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

Show a summary per file
File Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlAuthenticationProviderManager.cs Adds early return to protect user-defined providers and a new (incomplete) doc comment.
src/Microsoft.Data.SqlClient/tests/FunctionalTests/app.config Registers dummy authentication provider for .NET Framework tests.
src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj Ensures app.config is copied for netfx and includes dummy provider source.
src/Microsoft.Data.SqlClient/tests/FunctionalTests/DataCommon/DummySqlAuthenticationProvider.cs Adds dummy provider used to validate configuration-based registration.
src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlAuthenticationProviderTest.cs Adjusts default provider test and adds .NET Framework–specific override test.
src/Microsoft.Data.SqlClient/tests/FunctionalTests/AADAuthenticationTests.cs Adds test validating dummy provider registration via config.

Copy link

codecov bot commented Oct 3, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 75.61%. Comparing base (90bc83c) to head (b6eb430).
⚠️ Report is 7 commits behind head on release/6.0.

Files with missing lines Patch % Lines
...Data/SqlClient/SqlAuthenticationProviderManager.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           release/6.0    #3652      +/-   ##
===============================================
+ Coverage        72.73%   75.61%   +2.87%     
===============================================
  Files              285      244      -41     
  Lines            59162    40214   -18948     
===============================================
- Hits             43034    30409   -12625     
+ Misses           16128     9805    -6323     
Flag Coverage Δ
addons 92.58% <ø> (ø)
netcore 75.48% <0.00%> (-0.05%) ⬇️
netfx ?

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.

@cheenamalhotra cheenamalhotra merged commit c2184c6 into release/6.0 Oct 6, 2025
240 checks passed
@cheenamalhotra cheenamalhotra deleted the dev/cheena/6.0-fix-provider-registeration branch October 6, 2025 23:01
This was referenced Oct 8, 2025
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.

3 participants