Skip to content

Conversation

@ValentinVignal
Copy link
Contributor

Contributes to #130459

It adds a test for

  • examples/api/lib/material/icon_button/icon_button.3.dart

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels Jun 10, 2024
@goderbauer goderbauer requested review from TahaTesser and bleroux June 12, 2024 22:07
Comment on lines 16 to 20
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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

Copy link
Contributor Author

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

Copy link
Contributor

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?

Copy link
Contributor Author

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));
});

Copy link
Contributor

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.

@ValentinVignal ValentinVignal requested a review from bleroux June 13, 2024 08:22
Comment on lines 13 to 15
);
expect(find.widgetWithIcon(IconButton, Icons.settings_outlined), findsExactly(8));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
);
expect(find.widgetWithIcon(IconButton, Icons.settings_outlined), findsExactly(8));
);
expect(find.widgetWithIcon(IconButton, Icons.settings_outlined), findsExactly(8));

Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@bleroux bleroux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@TahaTesser TahaTesser added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 14, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented Jun 14, 2024

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.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 14, 2024
@bleroux bleroux force-pushed the flutter/api/example/Add-test-for-icon-button-3 branch from 248c614 to 6e26421 Compare June 16, 2024 16:21
@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 18, 2024
@auto-submit auto-submit bot merged commit f2c48af into flutter:master Jun 18, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 18, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 18, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 18, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 19, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 19, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 20, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 20, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 20, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jun 20, 2024
Manual roll requested by stuartmorgan@google.com

flutter/flutter@ccf3abe...6c06abb

2024-06-18 magder@google.com Add test for engine artifact framework permissions (flutter/flutter#148786)
2024-06-18 32538273+ValentinVignal@users.noreply.github.com Add test for icon_button.3.dart (flutter/flutter#149988)
2024-06-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 78fdd06af541 to 74f42ca3544c (6 revisions) (flutter/flutter#150421)
2024-06-18 tessertaha@gmail.com Fix transparent `dividerColor` breaks `TabBar.tabAlignment` (flutter/flutter#150350)
2024-06-18 tessertaha@gmail.com 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 jason-simmons@users.noreply.github.com Extend the Windows web_tool_tests_1_2 shard timeout to 45 minutes (flutter/flutter#150393)
2024-06-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1c4e5e230ecb to 78fdd06af541 (3 revisions) (flutter/flutter#150403)
2024-06-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from a4f266f7eb1a to 1c4e5e230ecb (8 revisions) (flutter/flutter#150399)
2024-06-18 parlough@gmail.com Rename doc file to use standard hyphens (flutter/flutter#150314)
2024-06-17 120297255+PurplePolyhedron@users.noreply.github.com Fix typo in `SliverLayoutDimensions.hashCode` where not all properties are used in the hash code. (flutter/flutter#150306)
2024-06-17 srawlins@google.com Fix doc comment references to 'this' (flutter/flutter#150379)
2024-06-17 Lexycon@users.noreply.github.com Add 'fail-fast' argument to flutter test (flutter/flutter#149587)
2024-06-17 dev@alestiago.com Update matchesGoldenFile documentation reference to goldenFileComparator (flutter/flutter#150343)
2024-06-17 dkwingsmt@users.noreply.github.com Reland 3: [CupertinoActionSheet] Match colors to native (flutter/flutter#150386)
2024-06-17 jhy03261997@gmail.com [a11y] Add semantics: button to bottom navigation bar items and dropdown menu items (flutter/flutter#149375)
2024-06-17 yinxulolol@gmail.com Reland "sliverGridDelegate mainAxisExtent add assert (#148470)"  (flutter/flutter#149720)
2024-06-17 36861262+QuncCccccc@users.noreply.github.com `ScaffoldBackgroundColor` should default to `ColorScheme.surface` (flutter/flutter#149772)
2024-06-17 katelovett@google.com Reland TreeSliver  (flutter/flutter#149839)
2024-06-17 dkwingsmt@users.noreply.github.com Reland: [CupertinoActionSheet] Add sliding tap gesture (flutter/flutter#150219)
2024-06-17 engine-flutter-autoroll@skia.org 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 rmistry@google.com,stuartmorgan@google.com 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
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants