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).

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

This PR updates the test utilities configuration workflow to support JSON-with-comments by moving the default test config to .jsonc, adjusting config-loading precedence, and enabling nullable reference types in the config model.

Changes:

Renames the default test config to config.default.jsonc and updates build logic to create/copy config.jsonc.
Updates Config.Load behavior to probe config locations in a new order and enables #nullable.
Updates ignore rules to exclude config.jsonc from source control.

Reviewed changes

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

File	Description
src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/Microsoft.Data.SqlClient.TestUtilities.csproj	Copies default config to `config.jsonc` and sets output-copy metadata.
src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/config.default.jsonc	Updates the sample config content and adds/adjusts fields.
src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/Config.cs	Refactors config load logic, adds jsonc/json probing, and enables nullable annotations.
.gitignore	Ignores generated `config.jsonc` files.

Comments suppressed due to low confidence (3)

src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/Config.cs:96

LoadInternal only catches FileNotFoundException. If TEST_MDS_CONFIG (or another attempted path) points to a non-existent directory, StreamReader will throw DirectoryNotFoundException and short-circuit the intended fallback to other locations. Consider treating DirectoryNotFoundException (and possibly other path-related IO exceptions) the same as file-not-found for the multi-location probe.

            try
            {
                using StreamReader sr = new StreamReader(configPath);
                return JsonConvert.DeserializeObject<Config>(sr.ReadToEnd()) ??
                       throw new InvalidOperationException($"Failed to deserialize config from '{configPath}'");
            }
            catch (FileNotFoundException)
            {
                // File did not exist at the path given. We will try a different location.
                return null;
            }

src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/Config.cs:76

UpdateConfig still defaults to writing config.json, but Load() now prefers config.jsonc when both exist. This can lead to updating one file while reads continue to use the other. Consider updating the default to config.jsonc, or writing to whichever config file was actually loaded/exists to avoid divergence.

        public static void UpdateConfig(Config updatedConfig, string configPath = @"config.json")
        {
            string config = JsonConvert.SerializeObject(updatedConfig);
            File.WriteAllText(configPath, config);

src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/config.default.jsonc:40

Possible typo in the sample config key: KerberosDomainuser doesn't match the KerberosDomainUser field name used in code. Aligning the casing/spelling will make the sample easier to follow (even if deserialization is case-insensitive).

+        public static Config Load(string configPath)
        {
-            // Allow an override of the config path via an environment variable.
-            configPath = Environment.GetEnvironmentVariable("TEST_MDS_CONFIG") ?? configPath;
+            Config config = LoadInternal(Environment.GetEnvironmentVariable("TEST_MDS_CONFIG")) ??
+                            LoadInternal(configPath) ??
+                            throw new FileNotFoundException("Could not find test configuration file.");



Copilot

Pull request overview

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

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/tests/tools/Microsoft.Data.SqlClient.TestUtilities/config.default.jsonc:40

Property name KerberosDomainuser has inconsistent casing vs the code/config consumers (KerberosDomainUser, e.g., Config.KerberosDomainUser and Kerberos pipeline steps). With System.Text.Json’s default case-sensitive matching this won’t bind as intended. Rename the JSON property to KerberosDomainUser (and keep casing consistent for other acronyms, e.g., URL).

+        {
+            if (configPath is null)
+            {
+                return null;
+            }
+


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


 You can override the config file path with the `MDS_TEST_CONFIG` environment variable:

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


Copilot

Pull request overview

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

 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.


 You can override the config file path with the `MDS_TEST_CONFIG` environment variable:

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

 On PowerShell:

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


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


Copilot

Pull request overview

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

+  # 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


          - 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
              foreach ($p in $jdata) {
                  $p.TCPConnectionString       = $env:REMOTE_TCP_CONN_STRING
                  $p.NPConnectionString        = $env:REMOTE_NP_CONN_STRING
                  $p.SupportsIntegratedSecurity = $true
                  $p.UseManagedSNIOnWindows    = $managedSni
              }
-              $jdata | ConvertTo-Json | Set-Content "config.json"
+              $jdata | ConvertTo-Json | Set-Content "config.jsonc"


  <Target Name="CopyConfig" BeforeTargets="Compile">
-    <Copy SourceFiles="config.default.json" DestinationFiles="config.json" Condition="!Exists('config.json')" />
+    <Copy SourceFiles="config.default.jsonc" DestinationFiles="config.jsonc" Condition="!Exists('config.jsonc')" />
  </Target>
  <ItemGroup>
-    <None Update="config.json">
-      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
-    </None>
-    <PackageReference Include="Newtonsoft.Json" />
+    <None Update="config.jsonc" CopyToOutputDirectory="PreserveNewest" />
+  </ItemGroup>


+            catch (FileNotFoundException)
+            {
+                // File did not exist at the path given. We will try a different location.


 You can override the config file path with the `MDS_TEST_CONFIG` environment variable:

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


 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.


Copilot

Pull request overview

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

+  # 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


          - 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
              foreach ($p in $jdata) {
                  $p.TCPConnectionString       = $env:REMOTE_TCP_CONN_STRING
                  $p.NPConnectionString        = $env:REMOTE_NP_CONN_STRING
                  $p.SupportsIntegratedSecurity = $true
                  $p.UseManagedSNIOnWindows    = $managedSni
              }
-              $jdata | ConvertTo-Json | Set-Content "config.json"
+              $jdata | ConvertTo-Json | Set-Content "config.jsonc"


 You can override the config file path with the `MDS_TEST_CONFIG` environment variable:

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

 On PowerShell:

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


+            catch (FileNotFoundException)
+            {
+                // File did not exist at the path given. We will try a different location.


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

Copilot AI reviewed May 20, 2026

View reviewed changes

🤖 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

Copilot AI reviewed May 20, 2026

View reviewed changes

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

Copilot AI reviewed May 20, 2026

View reviewed changes

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

Copilot AI reviewed May 21, 2026

View reviewed changes

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move Test Config to Jsonc#4297

Move Test Config to Jsonc#4297
benrr101 wants to merge 9 commits into
mainfrom
dev/russellben/config-jsonc

benrr101 commented May 20, 2026

Uh oh!

benrr101 May 20, 2026

Uh oh!

benrr101 May 20, 2026

Uh oh!

benrr101 May 20, 2026

Uh oh!

benrr101 May 20, 2026

Uh oh!

benrr101 May 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

benrr101 commented May 20, 2026

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants