Skip to content

Replace Newtonsoft.Json with System.Text.Json in tests and samples#4323

Draft
paulmedynski wants to merge 2 commits into
dev/russellben/config-jsoncfrom
dev/paul/newtonsoft
Draft

Replace Newtonsoft.Json with System.Text.Json in tests and samples#4323
paulmedynski wants to merge 2 commits into
dev/russellben/config-jsoncfrom
dev/paul/newtonsoft

Conversation

@paulmedynski
Copy link
Copy Markdown
Contributor

Description

Removes the Newtonsoft.Json dependency from all test and sample projects, replacing usage with System.Text.Json.

Based on #4297 which already migrated TestUtilities/Config.cs.

Changes

  • JsonBulkCopyTest / JsonStreamTest — migrated serialization and deep-comparison to System.Text.Json + new JsonTestHelper
  • JsonTestHelper — shared helper using JsonNode.DeepEquals on .NET 9+ and a recursive JsonElement comparison on .NET 8/462 (with max-depth guard)
  • SqlExceptionTest — rewrote to serialize key properties via STJ; removed JSONSerializationTest (tested Newtonsoft-specific ISerializable deserialization)
  • AzureKeyVaultProviderLegacyExample_2_0.cs — deleted (was already commented out and marked TODO for removal)
  • Project files — removed Newtonsoft.Json from ManualTests, FunctionalTests, Samples csprojs
  • Directory.Packages.props — removed Newtonsoft.Json PackageVersion entry

Follow-up

Testing

  • FunctionalTests SqlExceptionTest passes on net8.0, net9.0, net10.0
  • ManualTests builds clean on net8.0 and net9.0
  • Samples project builds clean
  • Zero remaining Newtonsoft references in *.cs, *.csproj, *.props

Note: Will re-target to main once #4297 is squash-merged.

Copilot AI review requested due to automatic review settings May 29, 2026 11:59
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board May 29, 2026
@paulmedynski paulmedynski added Area\Tests Issues that are targeted to tests or test projects Code Health 💊 Issues/PRs that are targeted to source code quality improvements. labels May 29, 2026
@paulmedynski paulmedynski added this to the 7.1.0-preview2 milestone May 29, 2026
Copy link
Copy Markdown
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

Removes the Newtonsoft.Json dependency from all test and sample projects in favor of System.Text.Json, deletes a long-commented legacy AKV sample, and drops the central Newtonsoft.Json PackageVersion. The PR also bundles a number of unrelated pipeline, flaky-test, and unit-test reliability changes that are not described in the PR text.

Changes:

  • Migrate JsonBulkCopyTest, JsonStreamTest, and SqlExceptionTest to STJ; add shared JsonTestHelper with JsonNode.DeepEquals on .NET 9+ and a recursive JsonElement fallback (depth-limited) on earlier TFMs.
  • Remove Newtonsoft.Json PackageReferences from ManualTests/FunctionalTests/Samples csprojs, remove its central PackageVersion, and delete AzureKeyVaultProviderLegacyExample_2_0.cs.
  • Unrelated extras: reset cached switches in LocalAppContextSwitchesTest, mark several simulated-server tests flaky, gate AsyncCancelledConnectionsTest against Kerberos/Managed Instance, add retryCountOnTaskFailure to dotnet install tasks, switch OneBranch symbols variable group/casing, add restore-dotnet-tools.yml to OneBranch and Kerberos pipelines, and change Kerberos code-coverage stage condition.

Reviewed changes

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

Show a summary per file
File Description
Directory.Packages.props Drop central Newtonsoft.Json PackageVersion.
doc/samples/Microsoft.Data.SqlClient.Samples.csproj Remove Newtonsoft.Json PackageReference.
doc/samples/AzureKeyVaultProviderLegacyExample_2_0.cs Delete already-commented legacy AKV sample.
src/.../tests/FunctionalTests/Microsoft.Data.SqlClient.FunctionalTests.csproj Remove Newtonsoft.Json PackageReference for net8/net462.
src/.../tests/FunctionalTests/SqlExceptionTest.cs Replace Newtonsoft round-trip with STJ property-projection serialization; remove JSONSerializationTest.
src/.../tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj Add JsonTestHelper.cs; drop Newtonsoft.Json PackageReferences.
src/.../tests/ManualTests/SQL/JsonTest/JsonBulkCopyTest.cs Switch JSON serialization and file comparison to STJ + JsonTestHelper.
src/.../tests/ManualTests/SQL/JsonTest/JsonStreamTest.cs Same STJ migration for the stream test.
src/.../tests/ManualTests/SQL/JsonTest/JsonTestHelper.cs New depth-limited deep-equals helper (delegates to JsonNode.DeepEquals on .NET 9+).
src/.../tests/ManualTests/DataCommon/DataTestUtility.cs Add IsNotKerberosTest helper used by gated tests.
src/.../tests/ManualTests/SQL/AsyncTest/AsyncCancelledConnectionsTest.cs Gate CancelAsyncConnections against MI and Kerberos environments.
src/.../tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs Reset cached switch fields via helper before asserting defaults.
src/.../tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs Mark two transient-fault retry-disabled tests flaky.
src/.../tests/UnitTests/SimulatedServerTests/ConnectionRoutingTestsAzure.cs Mark routed-location timeout test flaky.
eng/pipelines/steps/install-dotnet.yml Add retryCountOnTaskFailure: 3 to dotnet install tasks.
eng/pipelines/onebranch/variables/onebranch-variables.yml Switch symbols group to symbols-variables-v3 with Ppe casing.
eng/pipelines/onebranch/sqlclient-non-official.yml Use new SymbolsPublishServerPpe / SymbolsPublishTokenUriPpe.
eng/pipelines/onebranch/jobs/build-buildproj-job.yml Restore dotnet local tools before build.
eng/pipelines/kerberos/sqlclient-kerberos.yml Restore dotnet local tools; tighten coverage stage condition to succeeded().

Comment thread eng/pipelines/kerberos/sqlclient-kerberos.yml Outdated
- Migrate JsonBulkCopyTest and JsonStreamTest to System.Text.Json
- Add JsonTestHelper with DeepEquals supporting net8.0+ and net9.0+
- Rewrite SqlExceptionTest to serialize key properties via STJ
- Remove JSONSerializationTest (tested Newtonsoft-specific ISerializable)
- Delete legacy AzureKeyVaultProviderLegacyExample_2_0.cs (marked TODO)
- Remove Newtonsoft.Json from all csproj files and Directory.Packages.props

Tracks: #4322
@paulmedynski paulmedynski force-pushed the dev/paul/newtonsoft branch from 4c5e6d7 to b842414 Compare May 29, 2026 12:08
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Issue #4322 tracks the corresponding MSDocs removal of references to this file's snippets.

Assert.Equal(e.ClientConnectionId, sqlEx.ClientConnectionId);
Assert.Equal(e.StackTrace, sqlEx.StackTrace);
}
// Serialize the properties we want to validate round-trip through JSON.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The old test was confirming that Newtonsoft round-tripping functioned, which isn't our responsibility. Now we're checking that specific fields from our SqlException can be serialized, which is still not really our responsibility, but is at least more targeted.

});

[Fact]
public void JSONSerializationTest()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This test was actually a Newtonsoft unit test, which isn't something we should be testing.

@paulmedynski paulmedynski moved this from To triage to In progress in SqlClient Board May 29, 2026
System.Text.Json serializes supplementary-plane characters (e.g. U+29E3D)
as \uD867\uDE3D escape sequences, while SQL Server returns them as literal
UTF-8. The DeepEqualsCore fallback compared GetRawText() which preserves
escaping differences. Add explicit JsonValueKind.String case using
GetString() which decodes both representations to the same .NET string.
Copilot AI review requested due to automatic review settings May 29, 2026 17:04
Copy link
Copy Markdown
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 9 out of 9 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area\Tests Issues that are targeted to tests or test projects Code Health 💊 Issues/PRs that are targeted to source code quality improvements.

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

2 participants