Closed
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR splits the monolithic TvpTest manual test suite into smaller, independently runnable baseline-based tests, and introduces Copilot prompt/skill infrastructure and a coverage-overlap analysis script to improve test focus and developer workflows.
Changes:
- Refactors
TvpTestbaseline comparison to work against multiple split baseline files and exposes specific TVP test groups (column boundaries and query hints) via new internal methods. - Adds dedicated manual test classes and individual
.bslbaseline files for stream input parameters, TVP column boundaries, TVP query hints, SQL variant parameters, DateTime variants, and output parameters, wiring them into the manual test project. - Introduces
dotnet-coverageas a local tool plus a PowerShell script and Copilot prompt/skill files to analyze and reduce test overlap and to help generate MSTest filters and other agent workflows.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs | Updates RunTestCoreAndCompareWithBaseline to concatenate multiple baseline files, introduces FindDiffFromBaseline(string[] ...), makes CarriageReturnLineFeedReplacer, ColumnBoundariesTest, and QueryHintsTest internal to be reusable by new test classes. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHints_ReleaseMode_Azure.bsl | New baseline file for TVP query hints output in Release mode against Azure; content matches QueryHintsTest output including intentional DEFUALT token. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHints_ReleaseMode.bsl | New baseline for TVP query hints in Release mode on non-Azure targets. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHints_DebugMode_Azure.bsl | New baseline for TVP query hints in Debug mode on Azure targets. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHints_DebugMode.bsl | New baseline for TVP query hints in Debug mode on non-Azure targets. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpQueryHintsTests.cs | New manual test class that runs TvpTest.QueryHintsTest(), captures console output to TvpQueryHints.out, and compares against the appropriate TVP query hints .bsl file. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpColumnBoundaries_ReleaseMode_Azure.bsl | New baseline for TVP column boundaries output in Release mode on Azure; includes all iterations and error/output lines. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpColumnBoundaries_ReleaseMode.bsl | New baseline for TVP column boundaries output in Release mode (non-Azure). |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpColumnBoundaries_DebugMode_Azure.bsl | New Debug/Azure baseline for TVP column boundaries. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpColumnBoundaries_DebugMode.bsl | New Debug/non-Azure baseline for TVP column boundaries. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpColumnBoundariesTests.cs | New manual test class that calls TvpTest.ColumnBoundariesTest(), writes TvpColumnBoundaries.out, and compares it against the appropriate column-boundaries baseline file. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/StreamInputParameter_ReleaseMode_Azure.bsl | New short Release/Azure baseline for the tail of stream-input tests (async scope/cancel/command reuse). |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/StreamInputParameter_ReleaseMode.bsl | New short Release/non-Azure baseline for the stream-input test tail. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/StreamInputParameter_DebugMode_Azure.bsl | New Debug/Azure baseline containing the full stream-input parameter test output (large). |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/StreamInputParameter_DebugMode.bsl | New Debug/non-Azure baseline for stream-input parameter tests. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/StreamInputParameterTests.cs | New manual test class for StreamInputParam.Run, including the original "Starting test 'TvpTest'" header and baseline comparison via StreamInputParameter_*.bsl. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/SqlVariantParameter_ReleaseMode_Azure.bsl | New Release/Azure baseline capturing SQL variant behavior and expected money-vs-numeric mismatch diagnostics. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/SqlVariantParameter_ReleaseMode.bsl | New Release/non-Azure baseline for SQL variant parameter tests. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/SqlVariantParameter_DebugMode_Azure.bsl | New Debug/Azure baseline for SQL variant parameter tests. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/SqlVariantParameter_DebugMode.bsl | New Debug/non-Azure baseline for SQL variant parameter tests. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/SqlVariantParameterTests.cs | New manual test class that invokes SqlVariantParam.SendAllSqlTypesInsideVariant and asserts output against the appropriate SQL variant baseline. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/OutputParameter_ReleaseMode_Azure.bsl | New Release/Azure baseline for the small output parameter scenario. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/OutputParameter_ReleaseMode.bsl | New Release/non-Azure baseline file for output parameter tests. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/OutputParameter_DebugMode_Azure.bsl | New Debug/Azure baseline file for output parameter tests. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/OutputParameter_DebugMode.bsl | New Debug/non-Azure baseline for output parameter tests. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/OutputParameterTests.cs | New manual test class that runs OutputParameter.Run and compares console output to the new OutputParameter_*.bsl files. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/DateTimeVariantTests.cs | New manual test class that executes DateTimeVariantTest.TestAllDateTimeWithDataTypeAndVariant and validates output vs. DateTimeVariant_*.bsl baselines. |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpColumnBoundaries_ReleaseMode_Azure.bsl | (Duplicate path already described above in this table; ensures consistent Azure Release baseline for Tvp column boundaries.) |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpColumnBoundaries_ReleaseMode.bsl | (Duplicate path already described above; Release/non-Azure column boundaries baseline.) |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpColumnBoundaries_DebugMode_Azure.bsl | (Duplicate path already described above; Debug/Azure column boundaries baseline.) |
| src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpColumnBoundaries_DebugMode.bsl | (Duplicate path already described above; Debug/non-Azure column boundaries baseline.) |
| src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj | Wires in new test classes (*Tests.cs) and all new split baseline .bsl files as Content with CopyToOutputDirectory=PreserveNewest so they’re available at runtime. |
| dotnet-tools.json | Adds a local dotnet-coverage tool (version 18.3.2) definition for use in test coverage analysis workflows. |
| .github/skills/generate-mstest-filter/SKILL.md | New Copilot Agent skill that documents how to construct robust MSTest/xUnit filter expressions for dotnet test --filter. |
| .github/prompts/scripts/AnalyzeTestOverlap.ps1 | Adds a PowerShell script that builds a test project, runs filtered tests one-by-one under dotnet-coverage, parses XML coverage, and computes pairwise test overlap, emitting a JSON report and console summary. |
| .github/prompts/refine-test-overlap.prompt.md | New Copilot prompt that orchestrates running AnalyzeTestOverlap.ps1 and guiding refactoring decisions based on overlap results. |
| .github/prompts/plan-splitTvpTests.prompt.md | Planning document that describes the phased approach for splitting TvpTest into independent tests and baselines (largely corresponding to the work in this PR). |
| .github/prompts/generate-skill.prompt.md | Copilot prompt template for generating new SKILL.md agent skills following best practices. |
| .github/prompts/generate-prompt.prompt.md | Copilot prompt template for generating additional .prompt.md files, referencing skills when appropriate. |
| .github/prompts/ado-work-item-clarification.prompt.md | Copilot prompt for interactively clarifying and updating Azure DevOps work item descriptions. |
| .github/prompts/ado-work-item-agent.prompt.md | Copilot prompt that guides an engineer through the full lifecycle of addressing an ADO work item in this repo. |
| Console.Out.Dispose(); | ||
|
|
||
| // Recover the standard output stream | ||
| StreamWriter standardOutput = new(Console.OpenStandardOutput()); |
There was a problem hiding this comment.
Disposable 'StreamWriter' is created but not disposed.
| Console.Out.Dispose(); | ||
|
|
||
| // Recover the standard output stream | ||
| StreamWriter standardOutput = new(Console.OpenStandardOutput()); |
There was a problem hiding this comment.
Disposable 'StreamWriter' is created but not disposed.
| Console.Out.Dispose(); | ||
|
|
||
| // Recover the standard output stream | ||
| StreamWriter standardOutput = new(Console.OpenStandardOutput()); |
There was a problem hiding this comment.
Disposable 'StreamWriter' is created but not disposed.
| Console.Out.Dispose(); | ||
|
|
||
| // Recover the standard output stream | ||
| StreamWriter standardOutput = new(Console.OpenStandardOutput()); |
There was a problem hiding this comment.
Disposable 'StreamWriter' is created but not disposed.
Suggested change
| StreamWriter standardOutput = new(Console.OpenStandardOutput()); | |
| using StreamWriter standardOutput = new(Console.OpenStandardOutput()); |
| Console.Out.Dispose(); | ||
|
|
||
| // Recover the standard output stream | ||
| StreamWriter standardOutput = new(Console.OpenStandardOutput()); |
There was a problem hiding this comment.
Disposable 'StreamWriter' is created but not disposed.
Comment on lines
+34
to
+54
| string baselinePath; | ||
| #if DEBUG | ||
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | ||
| { | ||
| baselinePath = "OutputParameter_DebugMode.bsl"; | ||
| } | ||
| else | ||
| { | ||
| baselinePath = "OutputParameter_DebugMode_Azure.bsl"; | ||
| } | ||
| #else | ||
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | ||
| { | ||
| baselinePath = "OutputParameter_ReleaseMode.bsl"; | ||
| } | ||
| else | ||
| { | ||
| baselinePath = "OutputParameter_ReleaseMode_Azure.bsl"; | ||
| } | ||
| #endif | ||
|
|
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Suggested change
| string baselinePath; | |
| #if DEBUG | |
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | |
| { | |
| baselinePath = "OutputParameter_DebugMode.bsl"; | |
| } | |
| else | |
| { | |
| baselinePath = "OutputParameter_DebugMode_Azure.bsl"; | |
| } | |
| #else | |
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | |
| { | |
| baselinePath = "OutputParameter_ReleaseMode.bsl"; | |
| } | |
| else | |
| { | |
| baselinePath = "OutputParameter_ReleaseMode_Azure.bsl"; | |
| } | |
| #endif | |
| string mode = | |
| #if DEBUG | |
| "DebugMode"; | |
| #else | |
| "ReleaseMode"; | |
| #endif | |
| string azureSuffix = DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance ? string.Empty : "_Azure"; | |
| string baselinePath = $"OutputParameter_{mode}{azureSuffix}.bsl"; |
Comment on lines
+45
to
+52
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | ||
| { | ||
| baselinePath = "SqlVariantParameter_ReleaseMode.bsl"; | ||
| } | ||
| else | ||
| { | ||
| baselinePath = "SqlVariantParameter_ReleaseMode_Azure.bsl"; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Comment on lines
+37
to
+53
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | ||
| { | ||
| baselinePath = "StreamInputParameter_DebugMode.bsl"; | ||
| } | ||
| else | ||
| { | ||
| baselinePath = "StreamInputParameter_DebugMode_Azure.bsl"; | ||
| } | ||
| #else | ||
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | ||
| { | ||
| baselinePath = "StreamInputParameter_ReleaseMode.bsl"; | ||
| } | ||
| else | ||
| { | ||
| baselinePath = "StreamInputParameter_ReleaseMode_Azure.bsl"; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Suggested change
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | |
| { | |
| baselinePath = "StreamInputParameter_DebugMode.bsl"; | |
| } | |
| else | |
| { | |
| baselinePath = "StreamInputParameter_DebugMode_Azure.bsl"; | |
| } | |
| #else | |
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | |
| { | |
| baselinePath = "StreamInputParameter_ReleaseMode.bsl"; | |
| } | |
| else | |
| { | |
| baselinePath = "StreamInputParameter_ReleaseMode_Azure.bsl"; | |
| } | |
| baselinePath = DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance | |
| ? "StreamInputParameter_DebugMode.bsl" | |
| : "StreamInputParameter_DebugMode_Azure.bsl"; | |
| #else | |
| baselinePath = DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance | |
| ? "StreamInputParameter_ReleaseMode.bsl" | |
| : "StreamInputParameter_ReleaseMode_Azure.bsl"; |
Comment on lines
+38
to
+45
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | ||
| { | ||
| baselinePath = "TvpColumnBoundaries_ReleaseMode.bsl"; | ||
| } | ||
| else | ||
| { | ||
| baselinePath = "TvpColumnBoundaries_ReleaseMode_Azure.bsl"; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Comment on lines
+38
to
+45
| if (DataTestUtility.IsNotAzureServer() || DataTestUtility.IsManagedInstance) | ||
| { | ||
| baselinePath = "TvpQueryHints_ReleaseMode.bsl"; | ||
| } | ||
| else | ||
| { | ||
| baselinePath = "TvpQueryHints_ReleaseMode_Azure.bsl"; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Provide a summary of the changes being introduced. Important topics to cover
include:
High quality descriptions will lead to a smoother review experience.
Issues
Link to any relevant issues, bugs, or discussions (e.g.,
Closes #123,Fixes issue #456).Testing
Describe the automated tests (unit, integration) you created or modified.
Provide justification for any gap in automated testing. List any manual testing
steps that were performed to ensure the changes work.
Guidelines
Please review the contribution guidelines before submitting a pull request: