Add non-ASCII XML streaming tests for SqlDataReader.GetChars

[mdaigle]

Add tests that verify SqlDataReader.GetChars correctly handles non-ASCII characters when reading XML columns with SequentialAccess:

• GetChars_NonAsciiContent: Latin accented characters (2-byte UTF-8)
• GetChars_NonAsciiContent_BulkRead: Bulk read path with accented chars
• GetChars_CjkContent: CJK characters (3-byte UTF-8)

These tests establish a baseline for correct behavior on main before PR #3974 (issue #1877) refactors SqlStreamingXml internals.

[Copilot]

Pull request overview

This PR adds three new manual integration tests for SqlDataReader.GetChars to verify correct behavior when reading XML columns with CommandBehavior.SequentialAccess and non-ASCII Unicode content (Latin accented characters, CJK characters). These tests establish a behavioral baseline on main before PR #3974 refactors the SqlStreamingXml internals to fix an O(N²) performance issue (issue #1877).

Changes:

• New SqlStreamingXmlTest.cs with three test methods covering non-ASCII XML round-trips via GetChars (character-by-character and bulk read)
• Microsoft.Data.SqlClient.ManualTests.csproj updated to include the new test file

Reviewed changes

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

File	Description
SQL/SqlStreamingXmlTest/SqlStreamingXmlTest.cs	New test class with three [ConditionalFact] tests and a ReadAllChars helper
Microsoft.Data.SqlClient.ManualTests.csproj	Adds the new test file to the compile list

[Copilot]

The SqlDataReader returned by ExecuteReader is never disposed. If the assertions (or ReadAllChars) throw an exception, the reader is leaked. The established convention throughout the test suite is to wrap SqlDataReader in a using statement, e.g. using SqlDataReader sqlDataReader = command.ExecuteReader(CommandBehavior.SequentialAccess);. The same missing disposal applies to all three test methods (lines 29, 54, and 81).

[Copilot]

The SqlDataReader is not disposed — same issue as GetChars_NonAsciiContent. Wrap it in a using statement to ensure proper resource cleanup regardless of test outcome.

[Copilot]

The SqlDataReader is not disposed — same issue as GetChars_NonAsciiContent. Wrap it in a using statement to ensure proper resource cleanup regardless of test outcome.

[Copilot]

The text array in ReadAllChars is allocated with a fixed size (expectedSize). If the actual number of characters returned exceeds expectedSize, writing text[position] = buffer[0] will throw an IndexOutOfRangeException. While the + 10 buffer passed from the callers makes this unlikely for the specific test strings, there is no bounds check to protect against it. Consider either dynamically growing the buffer (e.g., using a List<char>) or adding a guard if (position < text.Length) before the array write.

Suggested change

	text[position] = buffer[0];
	position += numCharsRead;
	if (position < text.Length)
	{
	text[position] = buffer[0];
	position += numCharsRead;
	}
	else
	{
	// Prevent writing past the end of the fixed-size buffer.
	break;
	}

[Copilot]

The SqlConnection is created but not wrapped in a using statement. If an exception is thrown inside the using (SqlCommand ...) block before connection.Close() is called at line 36, the connection will never be disposed/closed, leaving a leaked connection. The established convention throughout the test suite is to declare SqlConnection with using, e.g. using SqlConnection connection = new(DataTestUtility.TCPConnectionString);. The same issue applies to all three test methods (lines 15, 43, and 70).

[Copilot]

The SqlConnection is created but not wrapped in a using statement (same issue as in GetChars_NonAsciiContent). If an exception is thrown before connection.Close() at line 63, the connection is leaked. Use using SqlConnection connection = new(DataTestUtility.TCPConnectionString); instead.

[Copilot]

The SqlConnection is created but not wrapped in a using statement (same issue as in the other two tests). If an exception is thrown before connection.Close() at line 88, the connection is leaked. Use using SqlConnection connection = new(DataTestUtility.TCPConnectionString); instead.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.26%. Comparing base (3303d80) to head (4ee4fec).
⚠️ Report is 4 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (3303d80) and HEAD (4ee4fec). Click for more details.

HEAD has 2 uploads less than BASE

Flag	BASE (3303d80)	HEAD (4ee4fec)
CI-SqlClient	2	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4005      +/-   ##
==========================================
- Coverage   74.38%   67.26%   -7.13%     
==========================================
  Files         287      282       -5     
  Lines       43982    67171   +23189     
==========================================
+ Hits        32717    45182   +12465     
- Misses      11265    21989   +10724     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	67.26% <ø> (?)

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.