Make sure that a CupertinoDesktopTextSelectionToolbarButton doesn't c… #173894
Conversation
…rash in 0x0 environment
There was a problem hiding this comment.
Code Review
This pull request adds a regression test for an issue where CupertinoDesktopTextSelectionToolbarButton would crash when rendered in a zero-sized area. The added test correctly reproduces the scenario. My feedback focuses on improving the test's robustness by adding an assertion, as recommended by the Flutter testing guidelines.
| await tester.pumpWidget( | ||
| const CupertinoApp( | ||
| home: SizedBox.shrink( | ||
| child: CupertinoDesktopTextSelectionToolbarButton(onPressed: null, child: Text('X')), | ||
| ), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
While this test is great for catching crashes, it's missing an assertion. The Flutter testing guide states that every test should have at least one expect call.1 This ensures the test is actually verifying what it's supposed to and doesn't pass vacuously if the widget isn't rendered at all.
Adding a simple expect to check for the widget's presence would make this test more complete.
await tester.pumpWidget(
const CupertinoApp(
home: SizedBox.shrink(
child: CupertinoDesktopTextSelectionToolbarButton(onPressed: null, child: Text('X')),
),
),
);
expect(find.byType(CupertinoDesktopTextSelectionToolbarButton), findsOneWidget);
Style Guide References
Footnotes
-
Flutter Style Guide, line 10: Writing Effective Tests. ↩
| await tester.pumpWidget( | ||
| const CupertinoApp( | ||
| home: SizedBox.shrink( | ||
| child: CupertinoDesktopTextSelectionToolbarButton(onPressed: null, child: Text('X')), | ||
| ), | ||
| ), | ||
| ); |
This is my attempt to handle #6537 for the CupertinoDesktopTextSelectionToolbarButton UI control.