PR Pipeline | Enable LocalDB Tests#4444
Conversation
…suggestion says this should cause conditional theory to run before the memebr data.
There was a problem hiding this comment.
Pull request overview
This PR updates the PR-validation pipeline to reliably execute LocalDB manual tests by adding a dedicated “localdb” manual test run (with blank connection strings and SupportsIntegratedSecurity=true) and adjusting a subset of manual tests to avoid discovery-time failures when connection strings are not configured.
Changes:
- Add a separate Windows-only PR pipeline job to run
TestSqlClientManualagainst LocalDB (blank TCP/NP connection strings + integrated security enabled), in parallel with existing localhost/azure runs. - Thread new pipeline parameters (
enableLocalDB,supportsIntegratedSecurity, LocalDB names) through PR pipeline templates and write them intoconfig.jsonc. - Update several ManualTests theories to guard on
AreConnStringsSetupand disable discovery enumeration forMemberDatato avoid “no data found” failures when connection strings are empty.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/ParametersTest.cs | Refactors scaled-decimal theory data generation and switches to ConditionalTheory/MemberData patterns for empty-conn-string scenarios. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/OutputParameterTests.cs | Adds AreConnStringsSetup gating to avoid running when no connection strings are configured. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MarsSessionPoolingTest.cs | Adds AreConnStringsSetup gating to avoid running when no connection strings are configured. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataStreamTest/DataStreamTest.cs | Disables discovery enumeration for connection-string-driven MemberData to prevent empty data set failures. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataStreamTest/CancellationTokenSourceDisposalTest.cs | Disables discovery enumeration for connection-string-driven MemberData to prevent empty data set failures. |
| eng/pipelines/pr/steps/configure-sqlserver-windows-step.yml | Adds conditional LocalDB enable/share/start step (parameter-driven). |
| eng/pipelines/pr/steps/configure-sqlserver-step.yml | Threads LocalDB-related parameters into Windows SQL Server configuration step. |
| eng/pipelines/pr/stages/test-stages.yml | Adds the new Windows-only LocalDB manual test job and sets integrated security flags per test configuration. |
| eng/pipelines/pr/jobs/test-sqlclientmanual-job.yml | Adds enableLocalDB + supportsIntegratedSecurity parameters and writes SupportsIntegratedSecurity into generated config.jsonc. |
…but it didn't matter b/c the test was marked as flaky.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4444 +/- ##
==========================================
- Coverage 65.83% 64.12% -1.72%
==========================================
Files 287 284 -3
Lines 43763 66786 +23023
==========================================
+ Hits 28812 42827 +14015
- Misses 14951 23959 +9008
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
As it turns out, the LocalDB tests were not being executed in the new sqlclient-pr pipeline. This is because the LocalDB tests require the localdb app names to be populated in config AND the supportsIntegratedAuth flag to be set to true. However, the supportsIntegratedAuth was being set to false because the connection strings for azure and localhost tests did not use integrated auth (azure uses managed identity, localhost uses username/password). The supportsIntegratedAuth flag is also mingled with other test such that just setting it to true causes failures with the aforementioned connection strings.
To resolve this and enable localdb testing, I've updated the test stages to include a separate run of manual test project with the connection strings left blank and the supportsIntegratedAuth set to true. This adds an extra 5-7min job to the stage which runs in parallel.
To enable this, I had to also modify some existing tests. There were some tests that would fail if the connection strings were empty. These fell into a couple categories:
AreConnStringsSetupconditional theory, but had member/class data that generated 1+ test cases per connection string. Since there were no connection strings, no tests were generated, resulting in a "no data available" test failure. Fixed by disabling enumeration at discovery (comments explaining why were wadded).AreConnStringsSetup. Most of these had some other condition that implied a connection (eg,IsNotAzureSynapse) but due to the implementation ended up being evaluated astrue(eg,!IsAzureSynapsewhich boils down to a check ofnull == 6, which meant!falsewhich istrue). Fixed by adding theAreConnStringsAddedcondition.In retrospect, it's possible that we could get localdb tests running as part of the azure manual test run. The error that came from enabling
supportsIntegratedAuthwas due to the connection string being parsed forpasswordfields. Since they do not exist on the azure connection strings, maybe they'd work for localdb testing.Testing