Add compatibility tests for ConvertTo-Json #26639
Conversation
iSazonov
added
the
CL-Test
|
I guess we can add more tests while working on V2 JSON serializer. So I will keep this open some time. |
There was a problem hiding this comment.
Pull request overview
This test-only PR adds compatibility tests for ConvertTo-Json to improve test coverage, extracted from PR #26637. While the intention is good, several of the new tests duplicate existing test coverage.
Key Points:
- Adds 14 new test cases for
ConvertTo-Jsoncmdlet - Several tests provide genuinely new coverage (Uri, enums, nested objects, EscapeNonAscii, depth validation)
- Multiple tests are redundant with existing test coverage (null handling, default escaping, HTML escaping, AsArray, Depth, multiple pipeline objects)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| It 'Should escape HTML with -EscapeHandling EscapeHtml' { | ||
| $json = @{ text = "<>&" } | ConvertTo-Json -Compress -EscapeHandling EscapeHtml | ||
| $json | Should -Match '\\u003C' | ||
| $json | Should -Match '\\u003E' | ||
| $json | Should -Match '\\u0026' | ||
| } |
There was a problem hiding this comment.
The HTML escape handling test is redundant. Lines 84-92 already test -EscapeHandling EscapeHtml with similar escaping validation. The existing test checks for \u0027 (escaped apostrophe), while this test checks for \u003C, \u003E, and \u0026 (escaped HTML characters), but the mechanism and parameter are already covered.
There was a problem hiding this comment.
Kept, but consolidated into TestCases format. Line 87 tests apostrophe escaping ('), while this tests HTML tag characters (<>&). Different character sets require explicit coverage.
test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Json.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Json.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Json.Tests.ps1
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Json.Tests.ps1
Outdated
Show resolved
Hide resolved
| It 'Should not escape by default' { | ||
| $json = @{ text = "<>&" } | ConvertTo-Json -Compress | ||
| $json | Should -BeExactly '{"text":"<>&"}' | ||
| } |
There was a problem hiding this comment.
The default escaping test is redundant. Lines 84-92 already test escaping behavior including:
- Default behavior (no escaping)
- Explicit
-EscapeHandling Default -EscapeHandling EscapeHtml
This test duplicates the existing coverage without adding new scenarios.
There was a problem hiding this comment.
Kept with renamed test. Line 85 tests apostrophe default behavior, while this tests HTML tag characters (<>&) default behavior. Since EscapeHtml specifically targets HTML characters, explicitly testing their default (non-escaped) behavior adds value.
|
If there are duplicates we could simply add comments in existing tests to describe additional scenarios are tested. If we can not create clear comments please keep duplicate tests. |
|
@iSazonov Sorry for the initial submission with redundant tests. I've refactored based on your feedback: Removed (covered by existing tests):
Consolidated into TestCases format:
Kept:
Added:
Result: +29/-51 lines, removing redundancy while improving edge case coverage. I'll add more test cases if I find other scenarios worth covering while working on V2 improvements. |
2 participants
PR Summary
Add compatibility tests for
ConvertTo-Jsonto improve test coverage.PR Context
This PR adds tests to validate
ConvertTo-Jsonbehavior, extracted from #26637 as suggested by @iSazonov.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerTests Added
The following tests are added to
ConvertTo-Json.Tests.ps1: