System.Text.Json Assertions for JsonNode, JsonElement, etc.

[thomhurst]

Fixes #4178

[github-actions]

PR Review: System.Text.Json Assertions

Reviewed PR #4179 - this is a solid implementation of JSON assertions for TUnit.

Strengths

Test Coverage

• Comprehensive positive and negative test cases
• Proper conditional compilation for NET8/NET9
• Tests verify error messages contain JSON paths
• Correct JsonDocument disposal with using statements

Code Quality

• Well-documented with XML comments
• JsonDiffHelper provides good separation of concerns
• Proper use of InlineMethodBody=true for performance
• Follows TUnit source-generated assertion patterns

Error Messages

• Detailed JSON path information (e.g., "$.person.age")
• Clear distinction between failure modes

Style

• Modern C# patterns (record struct, switch expressions)
• File-scoped namespaces
• Consistent naming

Areas for Improvement

1. Performance Notes

JsonStringAssertionExtensions creates JsonDocument on each call. This is acceptable for assertions (not a hot path), but consider documenting that users should prefer JsonElement/JsonNode assertions when working with already-parsed JSON.

2. Missing Edge Case Tests

Consider adding:

• Deeply nested JSON (verify path reporting)
• Unicode/special characters in property names
• More comprehensive empty object/array tests

3. Version Handling

Good job on version-specific conditionals:

• JsonNode.DeepEquals: NET8_0_OR_GREATER
• JsonElement.DeepEquals: NET9_0_OR_GREATER

4. Naming Consistency

The naming difference (IsJsonObject for JsonNode vs IsObject for JsonElement) is intentional and correct - it matches the underlying BCL types.

TUnit CLAUDE.md Checklist

• Rule 1 (Dual-Mode): Not applicable for assertion library
• Rule 2 (Snapshots): COMPLETED - API snapshots updated
• Rule 3 (VSTest): No VSTest references
• Rule 4 (Performance): Acceptable - simple assertions use InlineMethodBody
• Rule 5 (AOT): Compliant - no reflection, only BCL JSON APIs

Verdict

APPROVE with minor recommendations

No critical issues found. Code quality is excellent, test coverage is very good, and TUnit conventions are followed correctly. The recommendations above are nice-to-haves for future improvements.

Summary:

• Code quality: Excellent
• Test coverage: Very good
• Performance: Acceptable for assertion library
• Security: No concerns
• Could add edge case tests (non-blocking)

Great work!

[github-actions]

PR Review: System.Text.Json Assertions

This PR adds comprehensive JSON assertion capabilities to TUnit. Overall, this is a high-quality implementation that follows TUnit conventions well.

Strengths

1. Excellent Code Quality

Modern C# syntax with collection expressions, file-scoped namespaces, and pattern matching. Proper use of GenerateAssertion attribute. Clean separation of concerns. Good XML documentation throughout.

2. Smart Runtime Targeting

Proper use of conditional compilation for .NET versions. Leverages built-in DeepEquals methods instead of reimplementing. Appropriate feature gating based on API availability.

3. Helpful Error Messages

JsonDiffHelper provides path-to-difference error messages like differs at dollar-sign.age: expected 31 but found 30. This directly addresses the core issue requirement.

4. Good Test Coverage

Comprehensive test files for all assertion types with both positive and negative test cases. Tests verify error message content.

5. API Consistency

Snapshots updated for public API changes. Follows TUnit naming conventions.

Issues & Suggestions

CRITICAL: file modifier on extension classes

Location: JsonElementAssertionExtensions.cs:9, JsonNodeAssertionExtensions.cs:9, JsonStringAssertionExtensions.cs:9

The file modifier makes these extension methods invisible outside the declaring file. Users will not be able to call these assertions.

Fix Required: Change from file to public scope on all three extension classes. This is critical - the feature will not work without this change.

Performance: String allocation in diff path

Location: JsonDiffHelper.cs:51, 68

String interpolation allocates on every property even when there is no difference. Consider lazy path construction - only build path string if difference found. Low priority impact since diff checking is not a hot path.

Missing null handling test

Add test case for both JsonNode arguments being null to verify proper handling.

Naming inconsistency

JsonNode methods use Json prefix while JsonElement methods do not. Consider standardizing for consistency.

Recommended Actions

Required (Blocking):

1. Change file to public on all three extension classes

Recommended (Non-blocking):
2. Add test for IsDeepEqualTo with both arguments null
3. Add tests for deeply nested structures
4. Consider path string allocation optimization

Summary

This is excellent work that adds valuable JSON testing capabilities to TUnit. Once the file modifier issue is fixed, this PR will be ready to merge. Great job on the detailed error messages!

Reviewed using TUnit CLAUDE.md guidelines