Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Add comprehensive scalar type tests for ConvertTo-Json#26736

Merged
daxian-dbw merged 5 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom
yotsuda:add-convertto-json-scalar-tests-v2yotsuda/PowerShell:add-convertto-json-scalar-tests-v2Copy head branch name to clipboard
Feb 5, 2026
Merged

Add comprehensive scalar type tests for ConvertTo-Json#26736
daxian-dbw merged 5 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom
yotsuda:add-convertto-json-scalar-tests-v2yotsuda/PowerShell:add-convertto-json-scalar-tests-v2Copy head branch name to clipboard

Conversation

@yotsuda
Copy link
Contributor

@yotsuda yotsuda commented Feb 1, 2026

PR Summary

Add comprehensive scalar type tests for ConvertTo-Json (Phase 1).

PR Context

This is a follow-up to #26639. Per @iSazonov's suggestion, comprehensive tests are being submitted as separate PRs for each phase.

PR Checklist

Tests Added

The following scalar type tests are added to ConvertTo-Json.Tests.ps1:

Context Tests Description
Primitive scalar types 14 int, long, double, float, decimal, bool
String scalar types 9 regular, empty, spaces, newline, tab, quotes, backslash, unicode, emoji
DateTime and related types 5 UTC, Local, Unspecified, DateTimeOffset, TimeSpan
Guid type 3 Pipeline vs InputObject behavior
Uri type 4 http, https, query, file
Enum types 11 numeric values, -EnumsAsStrings, flags enum
IPAddress type 3 IPv4, IPv6, Extended properties
Pipeline vs InputObject 7 Validates consistent serialization for scalars
Scalars as elements 9 Arrays of scalars, mixed types, null elements
Scalars in hashtables 8 Hashtable and PSCustomObject with scalar values
ETS properties on scalars 4 string/DateTime ignore ETS, Uri/Guid include ETS

Total: 77 new tests

@yotsuda yotsuda requested a review from a team as a code owner February 1, 2026 14:21
iSazonov
iSazonov reviewed Feb 2, 2026
View reviewed changes
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
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 Show resolved Hide resolved
iSazonov
iSazonov reviewed Feb 2, 2026
View reviewed changes
Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

Please don't use Match if possible. I believe it is better use BeExactly.

test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Json.Tests.ps1 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
test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo-Json.Tests.ps1 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 Show resolved Hide resolved
@yotsuda
Copy link
Contributor Author

yotsuda commented Feb 2, 2026

Summary of All Changes

From first review:

  • Added Pipeline and InputObject checks to all type tests
  • Added ETS tests for all applicable types
  • Added NaN/Infinity edge cases for float/double
  • Added ETS tests for DateTimeOffset, DateOnly, TimeOnly
  • Added primitive types: byte, sbyte, short, ushort, uint, ulong
  • Changed -Match to -BeExactly where possible
  • Removed separate "Pipeline vs InputObject" Context
  • Consolidated array and hashtable tests using TestCases

From second review:

  • Added BigInteger to primitive types test cases
  • Changed all -Match to -BeExactly with full strings (DateOnly, TimeOnly, TimeSpan, IPAddress, ETS tests)
  • Removed redundant -Not -Match checks
  • Added ETS test for Enum type
  • Split IPv6 test into Pipeline and InputObject variants

Test results: 113 passed, 1 failed (pre-existing timezone issue), 1 pending

@iSazonov
Copy link
Collaborator

iSazonov commented Feb 2, 2026

@yotsuda Thanks! I still see Match is used 9 times in test. Can we replace them with BeExactly?

@yotsuda @claude
Replace -Match with -BeExactly and add $null test case
ef02f77 
Address review feedback by replacing all -Match assertions with -BeExactly
for more precise test validation. Also add missing $null test case to
Primitive scalar types.

Changes:
- Add $null test case to Primitive scalar types TestCases
- Replace -Match with -BeExactly in ETS properties tests (9 instances)
- Remove redundant -Not -Match assertion in String ETS test
- Calculate timezone offset dynamically for DateTime Local kind test
- Use full JSON string comparison for Guid array, Array ETS, and Hashtable ETS tests

All tests pass successfully (114 tests).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@yotsuda
Copy link
Contributor Author

yotsuda commented Feb 2, 2026

Changes Summary

I've addressed both review comments:

1. Replaced all -Match with -BeExactly (9 instances)

All tests now use -BeExactly for precise validation. The changes include:

  1. Primitive ETS properties (int/double): Added Expected parameter to TestCases for exact string comparison
  2. String ETS: Removed redundant -Not -Match 'MyProp' assertion (already covered by -BeExactly)
  3. DateTime Local kind: Calculate timezone offset dynamically for exact comparison
  4. Guid array: Full JSON string comparison including both "value" and "Guid" properties
  5. Array ETS: Full JSON string comparison
  6. Hashtable ETS: Full JSON string comparison

2. Added missing $null test case

Added a test case for $null in the Primitive scalar types TestCases:

# Null
@{ TypeName = 'null'; Value = $null; Expected = 'null' }

Test Results

All new and modified tests pass successfully:

  • ✅ 114 tests passed
  • ✅ $null serialization test added and passing
  • ✅ All -Match assertions replaced with -BeExactly

Note: There is 1 pre-existing test failure unrelated to these changes (DateTime timezone conversion issue in line 140).

Commit: ef02f77

@iSazonov iSazonov added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Feb 3, 2026
@iSazonov iSazonov self-assigned this Feb 3, 2026
@yotsuda @iSazonov
Update test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertTo…
5b8877e 
…-Json.Tests.ps1

Co-authored-by: Ilya <darpa@yandex.ru>
@yotsuda yotsuda mentioned this pull request Feb 3, 2026
Merged
6 tasks
@iSazonov iSazonov added the Approved-LowRisk Indicate a PR has been approved and can be merged after a quick review of another maintainer. label Feb 3, 2026
@daxian-dbw daxian-dbw merged commit dd4f2fe into PowerShell:master Feb 5, 2026
37 checks passed
@yotsuda yotsuda deleted the add-convertto-json-scalar-tests-v2 branch February 10, 2026 00:09
@yotsuda yotsuda mentioned this pull request Feb 10, 2026
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw daxian-dbw daxian-dbw approved these changes

@iSazonov iSazonov iSazonov approved these changes

Assignees

@iSazonov iSazonov

Labels

Approved-LowRisk Indicate a PR has been approved and can be merged after a quick review of another maintainer. CL-Test Indicates that a PR should be marked as a test change in the Change Log

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yotsuda @iSazonov @daxian-dbw

Comments

Close sidebar
Morty Proxy This is a proxified and sanitized view of the page, visit original site.