test(classes): add contract tests for class layer#62
Merged
Conversation
…atureFlag, and transform attributes Also fixes three bugs uncovered by the tests: - PropertyValidation: use Nullable[int] so unset constraints are not incorrectly applied - PropertyValidation.ToHashtable: omit null/unset fields to pass schema validation on round-trip - ConditionGroup constructor: treat null-valued keys as absent; use Property presence (not Operator) to detect flat vs group conditions so JSON round-trips do not falsely trigger mutual-exclusion or mixed-field errors - FeatureFlag constructor: handle Version deserialized as a dictionary (from ConvertTo-Json round-trip) instead of a version string Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Test Results 3 files 279 suites 5s ⏱️ Results for commit a26b910. |
There was a problem hiding this comment.
Pull request overview
Adds a new Pester contract test suite for Gatekeeper’s class layer (ConditionGroup/PropertySet/FeatureFlag and related transforms) and updates core class constructors/serialization helpers to support JSON round-trips and stricter validation semantics.
Changes:
- Added
tests/Classes.tests.ps1contract coverage for ConditionGroup, transform attributes, PropertyDefinition.Validate, PropertySet, and FeatureFlag. - Updated
PropertyValidationto use nullable constraint fields and omit unset fields during serialization to support schema-valid round-trips. - Updated
ConditionGroupandFeatureFlagconstructors to better handleConvertTo-Jsonround-trips (null/default field presence, Version shape).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/Classes.tests.ps1 | Adds class-layer contract tests covering constructors, validation, stringification, transforms, and save/load round-trips. |
| Gatekeeper/Classes/Property.ps1 | Fixes validation constraint defaults via nullable ints and omits unset validation fields in ToHashtable(). |
| Gatekeeper/Classes/FeatureFlag.ps1 | Adjusts ConditionGroup parsing rules for null/default fields and adds FeatureFlag Version dictionary handling. |
Comment on lines
+32
to
55
| # Property presence (non-null) is the canonical signal for a flat/leaf condition. | ||
| # Operator is excluded from this check because its enum default ("Equals") is always | ||
| # serialized as a non-null string in JSON round-trips, making it unreliable as a signal. | ||
| $hasGroup = $groupKeys.Count -gt 0 | ||
| $hasProperty = $data.ContainsKey('Property') -and $null -ne $data['Property'] | ||
| if ($hasGroup -and $hasProperty) { | ||
| throw "ConditionGroup with AllOf, AnyOf, or Not cannot also have Property, Operator, and Value defined." | ||
| } | ||
| if ($data.ContainsKey('Property') -and | ||
| (-not $data.ContainsKey('Operator') -or -not $data.ContainsKey('Value'))) { | ||
| throw "ConditionGroup with Property must also have Operator and Value defined." | ||
| if ($hasProperty) { | ||
| $hasOperator = $data.ContainsKey('Operator') -and $null -ne $data['Operator'] | ||
| $hasValue = $data.ContainsKey('Value') -and $null -ne $data['Value'] | ||
| if (-not $hasOperator -or -not $hasValue) { | ||
| throw "ConditionGroup with Property must also have Operator and Value defined." | ||
| } | ||
| } | ||
| if ($data.ContainsKey('Property')) { | ||
| if ($hasProperty) { | ||
| $this.Property = $data.Property | ||
| } | ||
| if ($data.ContainsKey('Operator')) { | ||
| if ($data.ContainsKey('Operator') -and $null -ne $data['Operator']) { | ||
| $this.Operator = $data.Operator | ||
| } | ||
| if ($data.ContainsKey('Value')) { | ||
| if ($data.ContainsKey('Value') -and $null -ne $data['Value']) { | ||
| $this.Value = $data.Value | ||
| } |
Comment on lines
+169
to
+180
| if ($null -ne $data.Version) { | ||
| if ($data.Version -is [System.Collections.IDictionary]) { | ||
| $b = [int]$data.Version.Build | ||
| $this.Version = if ($b -ge 0) { | ||
| [version]"$($data.Version.Major).$($data.Version.Minor).$b" | ||
| } else { | ||
| [version]"$($data.Version.Major).$($data.Version.Minor)" | ||
| } | ||
| } else { | ||
| $this.Version = [version]$data.Version | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #24.
Adds
tests/Classes.tests.ps1with contract tests forConditionGroup,ConditionGroupTransformAttribute,PropertyDefinition.Validate,PropertySet, andFeatureFlag.Also fixes four bugs exposed by the new tests:
[Nullable[int]]for integer constraint fields so unset constraints (MinLength,MaxLength, etc.) are not incorrectly applied during validation.ConvertTo-Jsonround-trips that serialize all class fields). UsePropertypresence (notOperator) as the canonical signal for a flat/leaf condition —Operator's enum default is always serialized as a non-null string, making it unreliable as a discriminator.Versiondeserialized as a dictionary from aConvertTo-Jsonround-trip rather than a version string.Test coverage added