Adds tests to ExpandAllGridItems and CollapseAllGridItems#14601
Adds tests to ExpandAllGridItems and CollapseAllGridItems#14601ricardobossan wants to merge 1 commit into
Conversation
- Not a bug. `PropertyGrid.ExpandAllGridItems` and `PropertyGrid.CollapseAllGridItems` had no unit test coverage, contributing to the PropertyGrid's overall coverage gap (~49.91%). - Add two unit tests to `PropertyGridTests.cs`: - `PropertyGrid_ExpandAllGridItems_ExpandsNestedGridItems` - `PropertyGrid_CollapseAllGridItems_CollapsesNestedGridItems` - Both reuse the existing `MyClass` fixture (it already exposes an `[ExpandableObjectConverter]` nested property), set it as `SelectedObject`, then assert the nested entry's `Expanded` state flips as expected. - Add a small `FindExpandableProperty` test helper that walks to the root grid item and returns the first expandable property entry. - None - No - Minimal - Unit tests - 11.0.100-preview.5.26227.104
|
Please fix the failed test cases first. |
| using PropertyGrid propertyGrid = new(); | ||
| propertyGrid.SelectedObject = new MyClass(); | ||
|
|
||
| GridItem nested = FindExpandableProperty(propertyGrid.SelectedGridItem); |
There was a problem hiding this comment.
This test doesn't assert the initial state before calling ExpandAllGridItems(). If nested.Expanded is already true by default (e.g., due to a future change in PropertyGrid initialization or the MyClass fixture), the test would pass vacuously without verifying that ExpandAllGridItems actually did anything.
Consider adding:
nested.Expanded.Should().BeFalse(); // Precondition: not yet expanded
propertyGrid.ExpandAllGridItems();
nested.Expanded.Should().BeTrue();The collapse test already has this pattern (it explicitly expands first, then asserts the collapse flips the state), which makes it a stronger test.
| { | ||
| return item; | ||
| } | ||
|
|
There was a problem hiding this comment.
The inner Find method returns null when no expandable item is found, but the outer FindExpandableProperty propagates that null through its non-nullable GridItem return type. While #nullable disable suppresses the warning, callers (the two tests above) rely on the .Should().NotBeNull() assertion as a guard.
This is fine today since MyClass has exactly one expandable property, but if MyClass is ever simplified or the [ExpandableObjectConverter] attribute is removed, the failure would manifest as a NullReferenceException on nested.Expanded rather than a clear "could not find expandable property" message.
Minor suggestion: consider throwing a descriptive exception (or using Assert.Fail) here instead of returning null, so a broken test fixture surfaces immediately with a clear message.
3 participants
Related to #12055
Root Cause
PropertyGrid.ExpandAllGridItemsandPropertyGrid.CollapseAllGridItemshad no unit test coverage, contributing to the PropertyGrid's overall coverage gap (~49.91%).Proposed changes
PropertyGridTests.cs:PropertyGrid_ExpandAllGridItems_ExpandsNestedGridItemsPropertyGrid_CollapseAllGridItems_CollapsesNestedGridItemsMyClassfixture (it already exposes an[ExpandableObjectConverter]nested property), set it asSelectedObject, then assert the nested entry'sExpandedstate flips as expected.FindExpandablePropertytest helper that walks to the root grid item and returns the first expandable property entry.Customer Impact
Regression?
Risk
Test methodology
Test environment(s)
Microsoft Reviewers: Open in CodeFlow