Skip to content

Add comprehensive scalar type tests for ConvertTo-Json#26736

Merged
daxian-dbw merged 5 commits intoPowerShell:masterfrom
yotsuda:add-convertto-json-scalar-tests-v2
Feb 5, 2026
Merged

Add comprehensive scalar type tests for ConvertTo-Json#26736
daxian-dbw merged 5 commits intoPowerShell:masterfrom
yotsuda:add-convertto-json-scalar-tests-v2

Conversation

@yotsuda
Copy link
Copy Markdown
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
iSazonov
iSazonov reviewed Feb 2, 2026
View reviewed changes
Copy link
Copy Markdown
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.

@yotsuda
Copy link
Copy Markdown
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
Copy Markdown
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 <[email protected]>
@yotsuda
Copy link
Copy Markdown
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 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