Repository navigation
Adds guideline tests to flutter create samples - #195827
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If both you and the PR author believe this PR qualifies for a test exemption, escalate to the tech lead for the changed area to review and apply the |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new accessibility guidelines test to the widget test template. The review feedback highlights that the test will fail because semantics are not enabled by default, and that applying both Android and iOS tap target guidelines unconditionally will cause conflicts. The feedback suggests enabling semantics and conditionally checking platform-specific guidelines.
| testWidgets('Meets accessibility guidelines', (WidgetTester tester) async { | ||
| // Build our app and trigger a frame. | ||
| await tester.pumpWidget(const MyApp()); | ||
|
|
||
| // Checks that tappable nodes have a minimum size of 48 by 48 pixels | ||
| // for Android. | ||
| await expectLater(tester, meetsGuideline(androidTapTargetGuideline)); | ||
|
|
||
| // Checks that tappable nodes have a minimum size of 44 by 44 pixels | ||
| // for iOS. | ||
| await expectLater(tester, meetsGuideline(iOSTapTargetGuideline)); | ||
|
|
||
| // Checks that touch targets with a tap or long press action are labeled. | ||
| await expectLater(tester, meetsGuideline(labeledTapTargetGuideline)); | ||
|
|
||
| // Checks that text meets the minimum contrast ratio: 4.5:1 for normal | ||
| // text and 3:1 for large text. | ||
| await expectLater(tester, meetsGuideline(textContrastGuideline)); | ||
| }); |
There was a problem hiding this comment.
There are two main issues with this new test:\n\n1. Semantics are not enabled: The meetsGuideline matcher requires the semantics tree to be active to evaluate accessibility guidelines. By default, widget tests do not enable semantics. Without calling tester.ensureSemantics(), this test will fail with a rejection indicating that semantics are not enabled.\n2. Conflicting/Redundant platform guidelines: Running both androidTapTargetGuideline (requires 48x48) and iOSTapTargetGuideline (requires 44x44) unconditionally on the same widget tree means the test will fail if any touch target is designed to iOS standards (e.g., 44x44) but falls short of Android's 48x48 requirement. We should conditionally apply these guidelines based on the target platform.
testWidgets('Meets accessibility guidelines', (WidgetTester tester) async {\n final SemanticsHandle handle = tester.ensureSemantics();\n try {\n // Build our app and trigger a frame.\n await tester.pumpWidget(const MyApp());\n\n // Checks that tappable nodes have a minimum size of 48 by 48 pixels\n // for Android, or 44 by 44 pixels for iOS.\n if (defaultTargetPlatform == TargetPlatform.android) {\n await expectLater(tester, meetsGuideline(androidTapTargetGuideline));\n } else if (defaultTargetPlatform == TargetPlatform.iOS) {\n await expectLater(tester, meetsGuideline(iOSTapTargetGuideline));\n }\n\n // Checks that touch targets with a tap or long press action are labeled.\n await expectLater(tester, meetsGuideline(labeledTapTargetGuideline));\n\n // Checks that text meets the minimum contrast ratio: 4.5:1 for normal\n // text and 3:1 for large text.\n await expectLater(tester, meetsGuideline(textContrastGuideline));\n } finally {\n handle.dispose();\n }\n });
There was a problem hiding this comment.
I'm not sure if this is true or not, but if it is then I guess the PR won't pass CI.
There was a problem hiding this comment.
This is inaccurate, we have had semantics turn on by default in test for a long long time. For some reason, AI keep thinking it is not on by default. I am going to look into where they got this idea.
I have tested the generated sample code and run the test locally to confirm it works
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM as long as the tests pass. Thanks for putting up the PR.
| testWidgets('Meets accessibility guidelines', (WidgetTester tester) async { | ||
| // Build our app and trigger a frame. | ||
| await tester.pumpWidget(const MyApp()); | ||
|
|
||
| // Checks that tappable nodes have a minimum size of 48 by 48 pixels | ||
| // for Android. | ||
| await expectLater(tester, meetsGuideline(androidTapTargetGuideline)); | ||
|
|
||
| // Checks that tappable nodes have a minimum size of 44 by 44 pixels | ||
| // for iOS. | ||
| await expectLater(tester, meetsGuideline(iOSTapTargetGuideline)); | ||
|
|
||
| // Checks that touch targets with a tap or long press action are labeled. | ||
| await expectLater(tester, meetsGuideline(labeledTapTargetGuideline)); | ||
|
|
||
| // Checks that text meets the minimum contrast ratio: 4.5:1 for normal | ||
| // text and 3:1 for large text. | ||
| await expectLater(tester, meetsGuideline(textContrastGuideline)); | ||
| }); |
There was a problem hiding this comment.
I'm not sure if this is true or not, but if it is then I guess the PR won't pass CI.
|
cc @paul-geoghegan as this was an action item from the Canonical summit's accessibility discussion. Once this lands, new Flutter apps will have a test that uses |
github-actions
Bot
added
the
tool
chunhtai
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/195827, because - This pull request has the |
|
Hi @bkonyi can i get a test exemption for this one? Reason: The change is actually a test that will be run in flutter/packages/flutter_tools/test/commands.shard/permeable/create_test.dart |
|
Seems great to me. Glad to have this in every new app. |
Approved! |
chunhtai
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
flutter/flutter@09175cd...e36b370 2026-11-07 [email protected] Roll Fuchsia Linux SDK from KIc0XZbhLmvDpTluZ... to Sf2yvdTiFXAKkfLkD... (flutter/flutter#196160) 2026-11-07 [email protected] Roll pub packages (flutter/flutter#196155) 2026-11-07 [email protected] [iOS a11y] Use a real `UIScrollView` as a11y scrollable container (flutter/flutter#195731) 2026-11-07 [email protected] Add ParagraphStyle.hyphenCharacter to dart:ui (flutter/flutter#195345) 2026-11-07 [email protected] Roll pub packages (flutter/flutter#196143) 2026-11-06 [email protected] [Linux] Fix layer position when compositing without glBlitFramebuffer (flutter/flutter#196083) 2026-11-06 [email protected] Add support for riscv64 desktop linux engine (flutter/flutter#178712) 2026-11-06 [email protected] Add ResizeImagePolicy.cover (flutter/flutter#194665) 2026-11-06 [email protected] Roll Skia from f533d4034900 to b11315d0a096 (6 revisions) (flutter/flutter#196138) 2026-11-06 [email protected] iOS: Remove dead code/unused imports from FlutterPlatformViewsController (flutter/flutter#196084) 2026-11-06 [email protected] Replace pull_request_target workflows with pull_request + workflow_run (flutter/flutter#195536) 2026-11-06 [email protected] [master] Update Flutter DEPS to Dart 3b174b7217fe09b21da836ea623932b5f041b091 (flutter/flutter#196004) 2026-11-06 [email protected] Fix android_hardware_smoke_test retry mechanisms (flutter/flutter#195708) 2026-11-06 [email protected] Roll Skia from 452b9c1e5db6 to f533d4034900 (1 revision) (flutter/flutter#196129) 2026-11-06 [email protected] [flutter_tools] Use `ErrorHandlingFileSystem.deleteIfExists` for `IOSCoreDeviceControl` temp directory cleanup (flutter/flutter#195913) 2026-11-06 [email protected] Adds guideline tests to flutter create samples (flutter/flutter#195827) 2026-11-06 [email protected] refactor(flutter_tools): unify OptionDescriptor identity and addTo (flutter/flutter#196076) 2026-11-06 [email protected] Roll pub packages (flutter/flutter#196114) 2026-11-06 [email protected] Roll Skia from 5ce64e63a0e7 to 452b9c1e5db6 (5 revisions) (flutter/flutter#196111) 2026-11-06 [email protected] Add PlatformDispatcher.onAlwaysUse24HourFormatChanged callback (flutter/flutter#194670) 2026-11-06 [email protected] Roll Packages from 0478162 to 63881f9 (10 revisions) (flutter/flutter#196108) 2026-11-06 [email protected] Roll Skia from 793b8ac73d4b to 5ce64e63a0e7 (4 revisions) (flutter/flutter#196107) If this roll has caused a breakage, revert this CL and set the roller to dry run mode 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
as title
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.