Move Test Config to Jsonc by benrr101 · Pull Request #4297 · dotnet/SqlClient

benrr101 · 2026-05-20T03:44:51Z

Description

As a byproduct of the PR pipeline, I got annoyed with the red squigglies under the comments in the test config.json file. Technically speaking jsonc files support comments while json does not. So, this small PR does the following:

Changes the config.default.json file to config.default.jsonc
Rewrites the Config.Load logic so that it tries the environment variable path first, config.jsonc second, then config.json before failing.
Makes Config class #nullable
Sync'd the config.default.json file with the fields that actually exist (and removed unused fields).
Remove dependency on Newtonsoft.Json from TestUtilities and push it back to ManualTests.

🤖

Used 🤖 to find remaining references to config.json and replace with references to config.jsonc

Issues

N/A

Testing

Everything builds, but I want to validate with a PR run before taking it out of draft state.

benrr101 · 2026-05-20T03:45:11Z

-        public string DNSCachingConnString = null;
-        public string DNSCachingServerCR = null;  // this is for the control ring
-        public string DNSCachingServerTR = null;  // this is for the tenant ring
-        public bool IsAzureSynapse = false; // True for Azure Data Warehouse/Synapse


This was removed as it is not used.

benrr101 · 2026-05-20T03:45:54Z

-        public string AliasName = null;

-        public static Config Load(string configPath = @"config.json")
+        public static Config Load(string configPath)


This was added as an escape hatch for preserving the behavior of ExtUtilities project. I have plans to remove the ExtUtilities project, so this method will hopefully go away someday.

benrr101 · 2026-05-20T03:48:11Z

+    "WorkloadIdentityFederationServiceConnectionId": "",
+    "KerberosDomainPassword": "",
+    "KerberosDomainuser": "",
+    "IsManagedInstance": false


These fields exist in Config class, but did not in the json file.

benrr101 · 2026-05-20T03:48:28Z

    "SupportsIntegratedSecurity": true,
    "LocalDbAppName": "",
    "LocalDbSharedInstanceName": "",
-    "SupportsFileStream": false,


This field did not exist in the Config class.

benrr101 · 2026-05-20T03:48:48Z

    "DNSCachingServerTR": "",
    "IsDNSCachingSupportedCR": false,
    "IsDNSCachingSupportedTR": false,
-    "IsAzureSynapse": false,


This field existed in the Config class, but was removed since it's unused.

Copilot

Pull request overview

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

Copilot

Pull request overview

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

-      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
-    </None>
-    <PackageReference Include="Newtonsoft.Json" />
+    <None Update="config.jsonc" CopyToOutputDirectory="PreserveNewest" />


+  # All properties should be added here, and this template should be used for any manipulation of the config.jsonc file.
  - pwsh: |
-      $jdata = Get-Content -Raw "config.default.json" | ConvertFrom-Json
+      $jdata = Get-Content -Raw "config.default.jsonc" | ConvertFrom-Json


          configSqlFor: local
          operatingSystem: Windows
-          # config.json properties
+          # config.jsonc properties


          - pwsh: |
              $managedSni = [System.Convert]::ToBoolean($env:MANAGED_SNI)
-              $jdata = Get-Content -Raw "config.default.json" | ConvertFrom-Json
+              $jdata = Get-Content -Raw "config.default.jsonc" | ConvertFrom-Json


          # --- Update test configuration (with Kerberos credentials) ---
          - pwsh: |
-              $jdata = Get-Content -Raw "config.default.json" | ConvertFrom-Json
+              $jdata = Get-Content -Raw "config.default.jsonc" | ConvertFrom-Json


 Edit the source configuration file at `src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/
-config.json`. The test utilities project copies that file to the test output directory, where the manual tests load it
+config.jsonc`. The test utilities project copies that file to the test output directory, where the manual tests load it
 by default.



 ```bash
-MDS_TEST_CONFIG=/path/to/config.json dotnet build -t:TestSqlClientManual -p:TestSet=2
+TEST_MDS_CONFIG=/path/to/config.jsonc dotnet build -t:TestSqlClientManual -p:TestSet=2


 ```powershell
-$env:MDS_TEST_CONFIG = "C:\path\to\config.json"
+$env:TEST_MDS_CONFIG = "C:\path\to\config.jsonc"
 dotnet build -t:TestSqlClientManual -p:TestSet=2


paulmedynski

Looks good, modulo Copilot feedback.

benrr101 added 2 commits May 19, 2026 18:27

Sync config.cs and config.json

025d441

Move config file to jsonc, make config class nullable

2d1aa7e

Copilot AI review requested due to automatic review settings May 20, 2026 03:44

github-project-automation Bot added this to SqlClient Board May 20, 2026

github-project-automation Bot moved this to To triage in SqlClient Board May 20, 2026

Copilot started reviewing on behalf of benrr101 May 20, 2026 03:45 View session

benrr101 commented May 20, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

🤖 updates to other references to config.json

0a0830b

edwardneal reviewed May 20, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/Config.cs Outdated

Replace newtonsoft with system.text.json

ffa9a9b

Copilot AI review requested due to automatic review settings May 20, 2026 17:15

Copilot started reviewing on behalf of benrr101 May 20, 2026 17:16 View session

Allow trailing commas and comments.

992fb31

benrr101 added this to the 7.1.0-preview2 milestone May 20, 2026

This comment was marked as outdated.

Sign in to view

Copilot AI review requested due to automatic review settings May 20, 2026 20:01

Copilot started reviewing on behalf of benrr101 May 20, 2026 20:01 View session

This comment was marked as outdated.

Sign in to view

Copilot AI review requested due to automatic review settings May 20, 2026 23:56

Copilot started reviewing on behalf of benrr101 May 20, 2026 23:57 View session

This comment was marked as outdated.

Sign in to view

benrr101 added 3 commits May 20, 2026 19:08

🤖 🌭

4474f38

Merge branch 'main' into dev/russellben/config-jsonc

5269fe9

Push newtonsoft reference back to manual tests

57e9b41

benrr101 force-pushed the dev/russellben/config-jsonc branch from 5885bb9 to 57e9b41 Compare May 21, 2026 00:09

Remove IsAzureSynapse from pipelines

ef6e8df

Copilot AI review requested due to automatic review settings May 21, 2026 15:51

Copilot started reviewing on behalf of benrr101 May 21, 2026 15:52 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

Comment thread eng/pipelines/common/templates/steps/update-config-file-step.yml

Comment thread eng/pipelines/kerberos/sqlclient-kerberos.yml

Comment thread TESTGUIDE.md Outdated

Comment thread src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/Config.cs Outdated

github-project-automation Bot moved this from In review to Waiting for customer in SqlClient Board Jun 2, 2026

paulmedynski added the Author attention needed PRs that require author to respond or make updates to PR. label Jun 2, 2026

Copilot comments.

bc3a0d8

Copilot AI review requested due to automatic review settings June 2, 2026 16:32

Copilot started reviewing on behalf of benrr101 June 2, 2026 16:33 View session

Remove lingering SupportsFileStream

48df8fc

benrr101 removed the Author attention needed PRs that require author to respond or make updates to PR. label Jun 2, 2026

benrr101 requested a review from paulmedynski June 2, 2026 16:35

This comment was marked as outdated.

Sign in to view

How many more comments will copilot give me this time?

06389a6

paulmedynski previously approved these changes Jun 2, 2026

View reviewed changes

paulmedynski assigned mdaigle Jun 3, 2026

mdaigle previously approved these changes Jun 3, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj

paulmedynski assigned cheenamalhotra Jun 3, 2026

paulmedynski requested a review from cheenamalhotra June 3, 2026 18:15

paulmedynski added the Author attention needed PRs that require author to respond or make updates to PR. label Jun 4, 2026

Merge branch 'main' into dev/russellben/config-jsonc

9502811

Copilot AI review requested due to automatic review settings June 4, 2026 17:09

benrr101 dismissed stale reviews from mdaigle and paulmedynski via 9502811 June 4, 2026 17:09

Copilot started reviewing on behalf of benrr101 June 4, 2026 17:09 View session

benrr101 moved this from Waiting for customer to In review in SqlClient Board Jun 4, 2026

benrr101 removed the Author attention needed PRs that require author to respond or make updates to PR. label Jun 4, 2026

Copilot AI reviewed Jun 4, 2026

View reviewed changes

paulmedynski approved these changes Jun 4, 2026

View reviewed changes

mdaigle approved these changes Jun 4, 2026

View reviewed changes

paulmedynski assigned apoorvdeshmukh Jun 5, 2026

paulmedynski requested a review from apoorvdeshmukh June 5, 2026 11:36

paulmedynski added the Author attention needed PRs that require author to respond or make updates to PR. label Jun 5, 2026

paulmedynski moved this from In review to Waiting for customer in SqlClient Board Jun 5, 2026

Conversation

benrr101 commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

🤖

Issues

Testing

Uh oh!

benrr101 May 20, 2026

Choose a reason for hiding this comment

Uh oh!

benrr101 May 20, 2026

Choose a reason for hiding this comment

Uh oh!

benrr101 May 20, 2026

Choose a reason for hiding this comment

Uh oh!

benrr101 May 20, 2026

Choose a reason for hiding this comment

Uh oh!

benrr101 May 20, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

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!

This comment was marked as outdated.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

paulmedynski left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

benrr101 commented May 20, 2026 •

edited

Loading