[cupertino_ui] Re-enable action_sheet_test.dart by justinmc · Pull Request #12055 · flutter/packages

justinmc · 2026-06-29T20:15:04Z

This file used TestSemantics from the Widgets library in flutter/flutter. This approach to semantics testing is not advised, as explained in flutter/flutter#184367. I migrated to the recommended tester.semantics.find.

The only use of TestSemantics in cupertino_ui. I refactored it to use the preferred tester.find.semantics instead.

gemini-code-assist

Code Review

This pull request removes the skip annotation from action_sheet_test.dart and refactors the 'Action sheet semantics' test to use tester.semantics.find and isSemantics instead of SemanticsTester. Feedback on the changes suggests restoring assertions for semantics actions (SemanticsAction.tap and SemanticsAction.focus) that were lost during the refactoring, and reusing sheetFinder to avoid redundant lookups.

gemini-code-assist · 2026-06-29T20:15:55Z

+    final Finder sheetFinder = find.bySemanticsLabel('Alert');
+    final SemanticsNode sheet = tester.semantics.find(find.bySemanticsLabel('Alert'));
+    expect(sheet.role, SemanticsRole.dialog);
+    expect(sheet, isSemantics(label: 'Alert', namesRoute: true, scopesRoute: true));
    expect(
-      semantics,
-      hasSemantics(
-        TestSemantics.root(
-          children: <TestSemantics>[
-            TestSemantics(
-              children: <TestSemantics>[
-                TestSemantics(
-                  children: <TestSemantics>[
-                    TestSemantics(
-                      flags: <SemanticsFlag>[SemanticsFlag.scopesRoute, SemanticsFlag.namesRoute],
-                      label: 'Alert',
-                      role: SemanticsRole.dialog,
-                      children: <TestSemantics>[
-                        TestSemantics(
-                          flags: <SemanticsFlag>[SemanticsFlag.hasImplicitScrolling],
-                          children: <TestSemantics>[
-                            TestSemantics(label: 'The title'),
-                            TestSemantics(label: 'The message'),
-                          ],
-                        ),
-                        TestSemantics(
-                          flags: <SemanticsFlag>[SemanticsFlag.hasImplicitScrolling],
-                          children: <TestSemantics>[
-                            TestSemantics(
-                              flags: <SemanticsFlag>[
-                                SemanticsFlag.isButton,
-                                SemanticsFlag.isFocusable,
-                              ],
-                              actions: <SemanticsAction>[
-                                SemanticsAction.tap,
-                                SemanticsAction.focus,
-                              ],
-                              label: 'One',
-                            ),
-                            TestSemantics(
-                              flags: <SemanticsFlag>[
-                                SemanticsFlag.isButton,
-                                SemanticsFlag.isFocusable,
-                              ],
-                              actions: <SemanticsAction>[
-                                SemanticsAction.tap,
-                                SemanticsAction.focus,
-                              ],
-                              label: 'Two',
-                            ),
-                          ],
-                        ),
-                        TestSemantics(
-                          flags: <SemanticsFlag>[SemanticsFlag.isButton, SemanticsFlag.isFocusable],
-                          actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus],
-                          label: 'Cancel',
-                        ),
-                      ],
-                    ),
-                  ],
-                ),
-              ],
-            ),
-          ],
-        ),
-        ignoreId: true,
-        ignoreRect: true,
-        ignoreTransform: true,
-      ),
+      find.descendant(of: sheetFinder, matching: find.bySemanticsLabel('The title')),
+      findsOneWidget,
    );
-
-    semantics.dispose();
+    expect(
+      find.descendant(of: sheetFinder, matching: find.bySemanticsLabel('The message')),
+      findsOneWidget,
+    );
+    final SemanticsNode buttonOne = tester.semantics.find(
+      find.descendant(of: sheetFinder, matching: find.bySemanticsLabel('One')),
+    );
+    expect(buttonOne, isSemantics(isButton: true, isFocusable: true));
+    final SemanticsNode buttonTwo = tester.semantics.find(
+      find.descendant(of: sheetFinder, matching: find.bySemanticsLabel('Two')),
+    );
+    expect(buttonTwo, isSemantics(isButton: true, isFocusable: true));
+    final SemanticsNode buttonCancel = tester.semantics.find(
+      find.descendant(of: sheetFinder, matching: find.bySemanticsLabel('Cancel')),
+    );
+    expect(buttonCancel, isSemantics(isButton: true, isFocusable: true));


The original test verified that the buttons ('One', 'Two', and 'Cancel') had specific semantics actions associated with them (SemanticsAction.tap and SemanticsAction.focus). The migrated test currently only asserts isButton and isFocusable, which reduces the test coverage.

We should restore these assertions using the actions parameter of isSemantics to ensure we don't lose coverage for accessibility actions. Additionally, we can reuse sheetFinder instead of calling find.bySemanticsLabel('Alert') twice.

final Finder sheetFinder = find.bySemanticsLabel('Alert'); final SemanticsNode sheet = tester.semantics.find(sheetFinder); expect(sheet.role, SemanticsRole.dialog); expect(sheet, isSemantics(label: 'Alert', namesRoute: true, scopesRoute: true)); expect( find.descendant(of: sheetFinder, matching: find.bySemanticsLabel('The title')), findsOneWidget, ); expect( find.descendant(of: sheetFinder, matching: find.bySemanticsLabel('The message')), findsOneWidget, ); final SemanticsNode buttonOne = tester.semantics.find( find.descendant(of: sheetFinder, matching: find.bySemanticsLabel('One')), ); expect( buttonOne, isSemantics( label: 'One', isButton: true, isFocusable: true, actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus], ), ); final SemanticsNode buttonTwo = tester.semantics.find( find.descendant(of: sheetFinder, matching: find.bySemanticsLabel('Two')), ); expect( buttonTwo, isSemantics( label: 'Two', isButton: true, isFocusable: true, actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus], ), ); final SemanticsNode buttonCancel = tester.semantics.find( find.descendant(of: sheetFinder, matching: find.bySemanticsLabel('Cancel')), ); expect( buttonCancel, isSemantics( label: 'Cancel', isButton: true, isFocusable: true, actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.focus], ), );

This appears to be a hallucination; isSemantics has no actions parameter.

I'm speculating that isButton is enough and we don't need to verify the actions, but I'm looking for input from @chunhtai.

I think isSemantics has hasTapAction and hasFocusAction that we can leverage.

Good call, added in 2cca7c1

Renzo-Olivares · 2026-06-29T20:44:17Z

-                          ],
-                        ),
-                        TestSemantics(
-                          flags: <SemanticsFlag>[SemanticsFlag.hasImplicitScrolling],


I wonder if we should still capture SemanticsFlag.hasImplicitScrolling . And how much of the actual semantics tree shape we want to verify. This test was very strict with regards to the shape of the tree, do we still want to keep it strict?

I'm planning to defer to @chunhtai on this.

I originally thought the hasImplicitScrolling was irrelevant, but maybe you do want to verify that the semantics tree is aware of the scrolling... Maybe we should be verifying everything from the "label: 'Alert'" level and below?

Maybe we should be verifying everything from the "label: 'Alert'" level and below?

yes I think so

@chunhtai I've now updated it to test the whole tree, similar to before except with isSemantics instead of TestSemantics. Is that what you had in mind? Some questions:

It seems like it's not possible to test the role inside of isSemantics. Is there a better way to do that or do we need to add it to isSemantics?

The error messages are not very good. If I forget a child in children, the error message is type 'Null' is not a subtype of type 'String' in type cast.

Should I instead do a flat list of expects like in the original commit fe965a5? Or is there a better way here?

I have been testing role using:

tester.getSemantics(find.byType(CupertinoSlidingSegmentedControl<int>)).role, SemanticsRole.radioGroup, );

It seems like it's not possible to test the role inside of isSemantics. Is there a better way to do that or do we need to add it to isSemantics?

right we should make sure it support role

The error messages are not very good. If I forget a child in children, the error message is type 'Null' is not a subtype of type 'String' in type cast.

we should improve the error message

pr to add role support flutter/flutter#188825

pr to improve child mismatch flutter/flutter#188827

Renzo-Olivares · 2026-06-29T22:00:27Z

@@ -1727,8 +1722,6 @@ void main() {
  }, skip: isBrowser); // https://github.com/flutter/flutter/issues/56001

  testWidgets('Action sheet semantics', (WidgetTester tester) async {
-    final semantics = SemanticsTester(tester);


Should we use SemanticsHandle to ensure semantics is initialized like below (plus dispose at the end of the test)? I'm not sure how necessary this is but I've been doing it in the migrations I've done.

final SemanticsHandle handle = tester.ensureSemantics();

Done in 49a84da, but maybe @chunhtai can confirm if it's needed?

semantics is enabled by default for testwidget now, so you don't need to initialize the handle in test.

chunhtai

LGTM

@example

…er#188916) flutter/packages@e742106...420e135 2026-07-02 dkwingsmt@users.noreply.github.com [material_ui] Migrate api sample code to @example dartdoc directive (flutter/packages#12078) 2026-07-02 dkwingsmt@users.noreply.github.com [material_ui] Move over more API samples (flutter/packages#12092) 2026-07-01 katelovett@google.com [cupertino_ui] Migrate api sample code to @example dartdoc directive (flutter/packages#12063) 2026-07-01 katelovett@google.com [cupertino_ui] Move over more API samples (flutter/packages#12086) 2026-07-01 48776784+mackings@users.noreply.github.com [two_dimensional_scrollables] Fix TreeView horizontal hit testing (flutter/packages#11859) 2026-07-01 1063596+reidbaker@users.noreply.github.com [camera_android_camerax][tool] Integrate dart_code_linter for cyclomatic complexity checks (flutter/packages#11999) 2026-07-01 dinurymomshad.dev@gmail.com [cross_file] Add runnable example with main() (flutter/packages#11527) 2026-07-01 36861262+QuncCccccc@users.noreply.github.com [cupertino_ui] Enable `switch_test.dart` (flutter/packages#12076) 2026-07-01 jmccandless@google.com [cupertino_ui] Re-enable dialog_test.dart (flutter/packages#12057) 2026-07-01 jmccandless@google.com [cupertino_ui] Re-enable action_sheet_test.dart (flutter/packages#12055) 2026-07-01 rmolivares@renzo-olivares.dev [cupertino_ui] Migrate `bottom_tab_bar_test.dart` to `SemanticsHandle` (flutter/packages#12012) 2026-07-01 1063596+reidbaker@users.noreply.github.com [repo] Add comment style guideline to AGENTS.md (flutter/packages#12077) 2026-07-01 43054281+camsim99@users.noreply.github.com Adds pre-push readiness skill (flutter/packages#11935) 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-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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

Re-enable action_sheet_test.dart.

fe965a5

The only use of TestSemantics in cupertino_ui. I refactored it to use the preferred tester.find.semantics instead.

justinmc self-assigned this Jun 29, 2026

flutter-dashboard Bot added the CICD Run CI/CD label Jun 29, 2026

justinmc added this to Decoupling Design and Test cross-imports Review Queue Jun 29, 2026

github-actions Bot added triage-framework Should be looked at in framework triage p: cupertino_ui labels Jun 29, 2026

github-project-automation Bot moved this to Todo in Test cross-imports Review Queue Jun 29, 2026

gemini-code-assist Bot reviewed Jun 29, 2026

View reviewed changes

justinmc requested review from QuncCccccc, Renzo-Olivares, chunhtai and elliette June 29, 2026 20:16

This was referenced Jun 29, 2026

Move away from TestSemantics and towards fine grained semantics testing flutter/flutter#184367

Open

(Test cross-imports) Don't cross-import test utils flutter/flutter#182636

Open

justinmc changed the title ~~Re-enable action_sheet_test.dart~~ [cupertino_ui] Re-enable action_sheet_test.dart Jun 29, 2026

Renzo-Olivares reviewed Jun 29, 2026

View reviewed changes

Test button actions

2cca7c1

github-actions Bot removed the CICD Run CI/CD label Jun 29, 2026

Renzo-Olivares reviewed Jun 29, 2026

View reviewed changes

justinmc added 3 commits June 29, 2026 15:56

SemanticsHandle and formatting

49a84da

No ensureSemantics needed.

507538c

Test full semantics tree

9969edc

justinmc added the CICD Run CI/CD label Jun 30, 2026

chunhtai approved these changes Jul 1, 2026

View reviewed changes

justinmc added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App and removed CICD Run CI/CD labels Jul 1, 2026

auto-submit Bot merged commit 4997fb6 into flutter:main Jul 1, 2026
14 checks passed

github-project-automation Bot moved this from Todo to Done in Test cross-imports Review Queue Jul 1, 2026

github-project-automation Bot moved this to Done in Decoupling Design Jul 1, 2026

justinmc deleted the cross-imports-action-sheet branch July 1, 2026 21:52

engine-flutter-autoroll mentioned this pull request Jul 2, 2026

Roll Packages from e74210630ab8 to 420e13502995 (13 revisions) flutter/flutter#188916

Merged

Uh oh!

Conversation

justinmc commented Jun 29, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants