Add test cases for PropertyGrid.AddTab#14574
Conversation
There was a problem hiding this comment.
Pull request overview
Adds unit test coverage for PropertyGrid.AddTab, supporting the broader effort to improve PropertyGrid coverage from issue #12055.
Changes:
- Adds tests for adding a new tab type.
- Adds coverage for duplicate tab type insertion behavior.
- Adds coverage for associating one or multiple components with a tab.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using PropertyGrid control = new(); | ||
| int initialCount = control.PropertyTabs.Count; | ||
|
|
||
| control.AddTab(typeof(TestPropertyTab), PropertyTabScope.Static); |
There was a problem hiding this comment.
The test name says AddsTabAtEnd, but AddTab doesn't unconditionally add at the end — it inserts alphabetically among non-default/non-event tabs (see the string.Compare(tab.TabName, current.TabName, ...) loop in the implementation). This assertion only holds because "TestTabName" > "Properties" and the Properties tab is the only existing tab, so the alphabetical insertion point happens to coincide with initialCount.
If a future change adds another default tab whose name sorts after "TestTabName", or if the tab name changes, this test would break for the wrong reason. Consider either:
- Renaming to something like
AddsTabInAlphabeticalOrderand asserting position more explicitly, or - Just asserting that the tab exists in the collection (
Assert.Contains) without asserting the exact index, which is an implementation detail.
|
|
||
| // Adding the same tab type a second time should be a no-op for tab insertion. | ||
| control.AddTab(typeof(TestPropertyTab), PropertyTabScope.Static); | ||
|
|
There was a problem hiding this comment.
This test calls AddTab(typeof(TestPropertyTab), PropertyTabScope.Static) twice with no component. Looking at the implementation: when the tab already exists, AddTab just skips the insertion block and falls through to if (@object is not null)—which is false here (@object defaults to null). So this test is effectively verifying that a specific code path is a no-op, which is fine.
However, a more interesting duplicate-prevention scenario might be calling AddTab with PropertyTabScope.Component + a component the first time, then with PropertyTabScope.Static (no component) the second time, and verifying the tab count stays the same and the previously-attached component is still there. That would guard against accidental tab-replacement on scope mismatch.
Related #12055
Proposed changes
Microsoft Reviewers: Open in CodeFlow