Skip to content

Add comprehensive PSCustomObject tests for ConvertTo-Json#26743

Open
yotsuda wants to merge 2 commits intoPowerShell:masterfrom
yotsuda:add-convertto-json-pscustomobject-tests
Open

Add comprehensive PSCustomObject tests for ConvertTo-Json#26743
yotsuda wants to merge 2 commits intoPowerShell:masterfrom
yotsuda:add-convertto-json-pscustomobject-tests

Conversation

@yotsuda
Copy link
Contributor

@yotsuda yotsuda commented Feb 3, 2026 •
edited
Loading

PR Summary

Add comprehensive PSCustomObject tests for ConvertTo-Json (Phase 3).

PR Context

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

PR Checklist

Tests Added

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

Context Tests Description
PSCustomObject basic serialization 4 single/multiple properties, property order, null property
PSCustomObject with various property types 7 scalar, DateTime, Guid, enum, array, hashtable properties
Nested PSCustomObject 4 nested, deeply nested, depth limit, mixed nested types
PSCustomObject ETS properties 3 NoteProperty, ScriptProperty, multiple ETS
Array of PSCustomObject 3 array serialization, single without -AsArray, single with -AsArray

Total: 21 new tests

@yotsuda yotsuda requested a review from a team as a code owner February 3, 2026 14:21
@yotsuda yotsuda force-pushed the add-convertto-json-pscustomobject-tests branch from 05f4e31 to 852aca0 Compare February 6, 2026 06:08
@yotsuda yotsuda mentioned this pull request Feb 6, 2026
Merged
6 tasks
iSazonov
iSazonov reviewed Feb 10, 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.

@yotsuda Please resolve merge conflicts.

Not all tests follow "via Pipeline and InputObject" pattern.

I'd move tests for PowerShell classes in another PR since we need more tests. I don't see tests with array and dictionary fields at least.

@yotsuda yotsuda force-pushed the add-convertto-json-pscustomobject-tests branch from 852aca0 to a781f41 Compare February 10, 2026 06:06
Copilot AI review requested due to automatic review settings February 10, 2026 06:06
@yotsuda
Copy link
Contributor Author

yotsuda commented Feb 10, 2026

Thank you for the review, @iSazonov!

I've addressed all your feedback:

  1. Merge conflicts resolved - Rebased on latest master (includes Add comprehensive scalar type tests for ConvertTo-Json #26736, Add comprehensive array and dictionary tests for ConvertTo-Json #26742, Add comprehensive ConvertTo-Json depth and multilevel composition tests #26744).
  2. "via Pipeline and InputObject" pattern - All tests now follow this pattern consistently.
  3. PowerShell class tests removed - Moved them out of this PR. I'll submit them in a separate PR with additional coverage for array and dictionary fields as you suggested.

All 22 PSCustomObject tests pass. (a781f41)

Copilot AI reviewed Feb 10, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new “Phase 3” test block to expand ConvertTo-Json coverage for PSCustomObject serialization scenarios in the PowerShell Utility module tests.

Changes:

  • Add comprehensive PSCustomObject serialization tests (basic, type coverage, nesting/depth, ETS properties).
  • Add array-focused PSCustomObject tests including -AsArray behavior.

iSazonov
iSazonov reviewed Feb 10, 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.

@yotsuda Please update the PR title.

@yotsuda yotsuda changed the title Add comprehensive PSCustomObject and class tests for ConvertTo-Json Add comprehensive PSCustomObject tests for ConvertTo-Json Feb 10, 2026
@yotsuda yotsuda mentioned this pull request Feb 10, 2026
Open
6 tasks
@iSazonov iSazonov added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label Feb 10, 2026
@iSazonov iSazonov self-assigned this Feb 10, 2026
@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 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@iSazonov iSazonov iSazonov approved these changes

At least 1 approving review is required to merge this pull request.

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.

2 participants

@yotsuda @iSazonov