Skip to content

[cupertino_ui] Migrate route_test.dart to SemanticsHandle#11993

Merged
auto-submit[bot] merged 6 commits into
flutter:mainfrom
Renzo-Olivares:cupertino_ui_route_test_migration
Jul 1, 2026
Merged

[cupertino_ui] Migrate route_test.dart to SemanticsHandle#11993
auto-submit[bot] merged 6 commits into
flutter:mainfrom
Renzo-Olivares:cupertino_ui_route_test_migration

Conversation

@Renzo-Olivares

Copy link
Copy Markdown
Contributor

Part of flutter/flutter#182636 and flutter/flutter#188395

This PR:

  • Removed the cross-import of widgets/semantics_tester.dart. Replaced SemanticsTester with SemanticsHandle.
  • Removed @Skip annotation, all tests in this file has passed. semantics_tester.dart has existed in cupertino_ui, so we can directly import semantics_tester.dart;
  • Moved the file to test/ folder.

Pre-Review Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter].
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I followed [the version and CHANGELOG instructions], using [semantic versioning] and the [repository CHANGELOG style], or I have commented below to indicate which documented exception this PR falls under[^1].
  • I updated/added any relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1].
  • All existing and new tests are passing.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request unskips the tests in packages/cupertino_ui/test/route_test.dart and refactors them to use tester.ensureSemantics() and standard semantics matchers instead of SemanticsTester. There are no review comments, and I have no feedback to provide.

@Renzo-Olivares Renzo-Olivares marked this pull request as draft June 25, 2026 18:02
@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 25, 2026
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jun 25, 2026
@Renzo-Olivares Renzo-Olivares marked this pull request as ready for review June 25, 2026 18:23

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request removes a test skip annotation, replaces SemanticsTester with SemanticsHandle and standard semantics matchers, wraps dialog and popup presentation calls in unawaited, and replaces manual throw statements with fail(). The review feedback suggests utilizing addTearDown to guarantee that debugDefaultTargetPlatformOverride and SemanticsHandle are cleaned up if a test fails, allowing the removal of manual cleanup lines.

Comment on lines +2043 to +2044
debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
final semantics = SemanticsTester(tester);
final SemanticsHandle handle = tester.ensureSemantics();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Overriding debugDefaultTargetPlatformOverride without ensuring it is reset in an addTearDown block can leak the iOS platform override to subsequent tests if this test fails. Using addTearDown ensures that both the platform override and the SemanticsHandle are properly cleaned up regardless of test success or failure.

    debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
    addTearDown(() {
      debugDefaultTargetPlatformOverride = null;
    });
    final SemanticsHandle handle = tester.ensureSemantics();
    addTearDown(handle.dispose);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ensureSemantics is no longer needed per @chunhtai.

Comment on lines +2072 to +2075
expect(find.semantics.byLabel('Dismiss'), findsNothing);

debugDefaultTargetPlatformOverride = null;
semantics.dispose();
handle.dispose();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since debugDefaultTargetPlatformOverride and handle are now cleaned up automatically via addTearDown, these manual cleanup lines can be removed.

    expect(find.semantics.byLabel('Dismiss'), findsNothing);

Comment on lines +2081 to +2082
debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
final semantics = SemanticsTester(tester);
final SemanticsHandle handle = tester.ensureSemantics();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Overriding debugDefaultTargetPlatformOverride without ensuring it is reset in an addTearDown block can leak the iOS platform override to subsequent tests if this test fails. Using addTearDown ensures that both the platform override and the SemanticsHandle are properly cleaned up regardless of test success or failure.

    debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
    addTearDown(() {
      debugDefaultTargetPlatformOverride = null;
    });
    final SemanticsHandle handle = tester.ensureSemantics();
    addTearDown(handle.dispose);

Comment on lines +2111 to +2117
expect(
semantics,
includesNodeWith(
actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.dismiss],
label: 'Dismiss',
),
find.semantics.byLabel('Dismiss'),
isSemantics(label: 'Dismiss', hasTapAction: true, hasDismissAction: true),
);

debugDefaultTargetPlatformOverride = null;
semantics.dispose();
handle.dispose();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since debugDefaultTargetPlatformOverride and handle are now cleaned up automatically via addTearDown, these manual cleanup lines can be removed.

    expect(
      find.semantics.byLabel('Dismiss'),
      isSemantics(label: 'Dismiss', hasTapAction: true, hasDismissAction: true),
    );

@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 25, 2026
@Renzo-Olivares Renzo-Olivares added the CICD Run CI/CD label Jun 25, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 25, 2026
@Renzo-Olivares Renzo-Olivares added the CICD Run CI/CD label Jun 25, 2026

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM minus ensureSemantics 👍

child: Placeholder(),
);
},
unawaited(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I migrated all of these in the main tests but missed the ones in temporarily_disabled_tests, thanks for fixing.

Comment on lines +2043 to +2044
debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
final semantics = SemanticsTester(tester);
final SemanticsHandle handle = tester.ensureSemantics();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ensureSemantics is no longer needed per @chunhtai.

@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 30, 2026
@Renzo-Olivares Renzo-Olivares added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Jun 30, 2026
@auto-submit auto-submit Bot merged commit b791d2a into flutter:main Jul 1, 2026
88 checks passed
pull Bot pushed a commit to safarmer/flutter that referenced this pull request Jul 1, 2026
…er#188863)

flutter/packages@274ed3e...e742106

2026-07-01 rmolivares@renzo-olivares.dev [cupertino_ui] Re-enable
`tab_scaffold_test.dart` (flutter/packages#12064)
2026-07-01 jmccandless@google.com [material_ui] Port flutter/flutter
flutter#184808 "Remove semantics_tester import from card_test.dart"
(flutter/packages#11965)
2026-07-01 rmolivares@renzo-olivares.dev [cupertino_ui] Migrate
`sliding_segmented_control_test.dart` to `SemanticsHandle`
(flutter/packages#11979)
2026-07-01 rmolivares@renzo-olivares.dev [cupertino_ui] Migrate
`route_test.dart` to `SemanticsHandle` (flutter/packages#11993)
2026-07-01 rmolivares@renzo-olivares.dev [cupertino_ui] Migrate
`nav_bar_test.dart` to `SemanticsHandle` (flutter/packages#11980)
2026-07-01 rmolivares@renzo-olivares.dev [cupertino_ui] Migrate
`segmented_control_test.dart` to `SemanticsHandle`
(flutter/packages#11982)
2026-07-01 rmolivares@renzo-olivares.dev [cupertino_ui] Re-enable
`text_field_test.dart` (flutter/packages#12067)
2026-06-30 r.anantheswar@gmail.com [camera_android_camerax] Pass
targetVideoEncodingBitRate to Recorder (flutter/packages#11960)
2026-06-30 1063596+reidbaker@users.noreply.github.com
[camera_android_camerax] Migrate check-readiness skill from bash to Dart
(flutter/packages#11943)
2026-06-30 21270878+elliette@users.noreply.github.com [material_ui]
Enable `time_picker_test` (flutter/packages#12061)
2026-06-30 36861262+QuncCccccc@users.noreply.github.com [cupertino_ui]
Migrate checkbox_test.dart to SemanticsHandle (flutter/packages#12065)
2026-06-30 64674824+yashas-hm@users.noreply.github.com [image_picker]
Handle limit: 1 in pickMultiImage and pickMultipleMedia gracefully
(flutter/packages#11825)
2026-06-30 faheemabbas766@gmail.com [cross_file] Document native
mimeType behavior (flutter/packages#11662)
2026-06-30 36861262+QuncCccccc@users.noreply.github.com [material_ui]
Remove `image_data.dart` imports from `circle_avatar_test.dart`,
`color_scheme_test.dart` (flutter/packages#12059)
2026-06-30 36861262+QuncCccccc@users.noreply.github.com [cupertino_ui]
Remove `image_data.dart` import from `scaffold_test.dart`
(flutter/packages#12060)
2026-06-30 36861262+QuncCccccc@users.noreply.github.com [cupertino_ui]
Remove widgets import from menu_anchor_test.dart
(flutter/packages#12068)
2026-06-30 21270878+elliette@users.noreply.github.com [material_ui]
Enable `checkbox_list_tile_test` (flutter/packages#12007)
2026-06-30 louisehsu@google.com [in_app_purchase_storekit] Expose
quantity in Transactions (flutter/packages#11879)

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
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 CICD Run CI/CD p: cupertino_ui triage-framework Should be looked at in framework triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants