-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add test for icon_button.3.dart #149988
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add test for icon_button.3.dart #149988
Conversation
| for (int i = 0; i <= 6; i += 2) { | ||
| expect(tester.widget<IconButton>(unselectedIconButtons.at(i)).onPressed, isA<VoidCallback>()); | ||
| expect(tester.widget<IconButton>(unselectedIconButtons.at(i + 1)).onPressed, isNull); | ||
| } | ||
| expect(tester.widgetList<IconButton>(unselectedIconButtons).map((IconButton iconButton) => iconButton.isSelected), everyElement(isFalse)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| for (int i = 0; i <= 6; i += 2) { | |
| expect(tester.widget<IconButton>(unselectedIconButtons.at(i)).onPressed, isA<VoidCallback>()); | |
| expect(tester.widget<IconButton>(unselectedIconButtons.at(i + 1)).onPressed, isNull); | |
| } | |
| expect(tester.widgetList<IconButton>(unselectedIconButtons).map((IconButton iconButton) => iconButton.isSelected), everyElement(isFalse)); | |
| for (int i = 0; i <= 6; i++) { | |
| final IconButton button = tester.widget<IconButton>(unselectedIconButtons.at(i)); | |
| expect(button.onPressed, i.isEven ? isA<VoidCallback>() : isNull); | |
| expect(button.isSelected, isFalse); | |
| } |
Do you think this alternative is more readable? I prefer it because it avoids repeating the long conversion to IconButton and the mix of for and map. Up to you to adopt it or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, it looks simpler, I modified it in refactor: Better readable code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, it looks simpler, I modified it in refactor: Better readable code
Great! Can you apply the same to the other similar sections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops sorry, I pushed it in format: Better formatting
| } | ||
| expect(tester.widgetList<IconButton>(unselectedIconButtons).map((IconButton iconButton) => iconButton.isSelected), everyElement(isFalse)); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider removing this extra line.
| ); | ||
| expect(find.widgetWithIcon(IconButton, Icons.settings_outlined), findsExactly(8)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ); | |
| expect(find.widgetWithIcon(IconButton, Icons.settings_outlined), findsExactly(8)); | |
| ); | |
| expect(find.widgetWithIcon(IconButton, Icons.settings_outlined), findsExactly(8)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add extra line between pumpWidget and expect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it in format: Better formatting
bleroux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
auto label is removed for flutter/flutter/149988, due to - The status or check suite Mac tool_integration_tests_1_4 has failed. Please fix the issues identified (or deflake) before re-applying this label. |
248c614 to
6e26421
Compare
Manual roll requested by [email protected] flutter/flutter@ccf3abe...6c06abb 2024-06-18 [email protected] Add test for engine artifact framework permissions (flutter/flutter#148786) 2024-06-18 [email protected] Add test for icon_button.3.dart (flutter/flutter#149988) 2024-06-18 [email protected] Roll Flutter Engine from 78fdd06af541 to 74f42ca3544c (6 revisions) (flutter/flutter#150421) 2024-06-18 [email protected] Fix transparent `dividerColor` breaks `TabBar.tabAlignment` (flutter/flutter#150350) 2024-06-18 [email protected] Fix scrollable `TabBar` jittering (flutter/flutter#150041) 2024-06-18 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland 3: [CupertinoActionSheet] Match colors to native (#150386)" (flutter/flutter#150413) 2024-06-18 [email protected] Extend the Windows web_tool_tests_1_2 shard timeout to 45 minutes (flutter/flutter#150393) 2024-06-18 [email protected] Roll Flutter Engine from 1c4e5e230ecb to 78fdd06af541 (3 revisions) (flutter/flutter#150403) 2024-06-18 [email protected] Roll Flutter Engine from a4f266f7eb1a to 1c4e5e230ecb (8 revisions) (flutter/flutter#150399) 2024-06-18 [email protected] Rename doc file to use standard hyphens (flutter/flutter#150314) 2024-06-17 [email protected] Fix typo in `SliverLayoutDimensions.hashCode` where not all properties are used in the hash code. (flutter/flutter#150306) 2024-06-17 [email protected] Fix doc comment references to 'this' (flutter/flutter#150379) 2024-06-17 [email protected] Add 'fail-fast' argument to flutter test (flutter/flutter#149587) 2024-06-17 [email protected] Update matchesGoldenFile documentation reference to goldenFileComparator (flutter/flutter#150343) 2024-06-17 [email protected] Reland 3: [CupertinoActionSheet] Match colors to native (flutter/flutter#150386) 2024-06-17 [email protected] [a11y] Add semantics: button to bottom navigation bar items and dropdown menu items (flutter/flutter#149375) 2024-06-17 [email protected] Reland "sliverGridDelegate mainAxisExtent add assert (#148470)" (flutter/flutter#149720) 2024-06-17 [email protected] `ScaffoldBackgroundColor` should default to `ColorScheme.surface` (flutter/flutter#149772) 2024-06-17 [email protected] Reland TreeSliver (flutter/flutter#149839) 2024-06-17 [email protected] Reland: [CupertinoActionSheet] Add sliding tap gesture (flutter/flutter#150219) 2024-06-17 [email protected] Roll Flutter Engine from 5989f0215fed to a4f266f7eb1a (1 revision) (flutter/flutter#150377) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Contributes to #130459
It adds a test for
examples/api/lib/material/icon_button/icon_button.3.dartPre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.