Remove semantics_tester import from card_test.dart by Sanaullah49 · Pull Request #184808 · flutter/flutter

Sanaullah49 · 2026-04-09T07:35:58Z

Part of #182636

Summary

Remove the semantics_tester.dart cross-import from card_test.dart.

What changed

Replaced the deprecated SemanticsTester/TestSemantics assertions in the Card semantics tests with direct semantics lookups and matchesSemantics(...)
Kept the existing coverage for separate child semantics and merged semantic container behavior
Removed the ../widgets/semantics_tester.dart import

Testing

../../bin/flutter analyze test/material/card_test.dart
../../bin/flutter test test/material/card_test.dart

gemini-code-assist

Code Review

This pull request refactors card_test.dart to use SemanticsHandle and matchesSemantics for semantics testing. The feedback recommends using addTearDown for SemanticsHandle disposal to ensure test isolation and removing the manual disposal calls.

gemini-code-assist · 2026-04-09T07:40:19Z


  testWidgets('Card can take semantic text from multiple children', (WidgetTester tester) async {
-    final semantics = SemanticsTester(tester);
+    final SemanticsHandle handle = tester.ensureSemantics();


In Flutter tests, it is a best practice to use addTearDown to ensure that resources like SemanticsHandle are properly disposed of even if the test fails. This prevents semantics from remaining enabled and potentially causing flakiness or failures in subsequent tests. Since the handle is not used for anything other than disposal in this test, you can simplify the implementation.

Suggested change

final SemanticsHandle handle = tester.ensureSemantics();

addTearDown(tester.ensureSemantics().dispose);

References

The Flutter repository follows the 'Writing Effective Tests' guide, which advocates for using addTearDown for reliable resource cleanup to ensure test isolation. ^(link)

Yes, rather than your try / finally approach, use addTearDown

gemini-code-assist · 2026-04-09T07:40:19Z

    );

-    semantics.dispose();
+    handle.dispose();


This manual disposal is redundant if addTearDown is used at the beginning of the test.

gemini-code-assist · 2026-04-09T07:40:19Z

  testWidgets('Card merges children when it is a semanticContainer', (WidgetTester tester) async {
-    final semantics = SemanticsTester(tester);
-    debugResetSemanticsIdCounter();
+    final SemanticsHandle handle = tester.ensureSemantics();


As with the previous test, using addTearDown for the SemanticsHandle is the preferred pattern for robust cleanup in the Flutter repository.

Suggested change

final SemanticsHandle handle = tester.ensureSemantics();

addTearDown(tester.ensureSemantics().dispose);

References

The Flutter repository follows the 'Writing Effective Tests' guide, which advocates for using addTearDown for reliable resource cleanup to ensure test isolation. ^(link)

gemini-code-assist · 2026-04-09T07:40:20Z

    );

-    semantics.dispose();
+    handle.dispose();


This manual disposal is redundant if addTearDown is used.

Sanaullah49 · 2026-04-28T10:53:16Z

Updated the semantics-handle cleanup to dispose in finally, which preserves cleanup if the test body fails while still satisfying Flutter test binding verification that handles are closed before test teardown. Verified locally: ./bin/flutter test packages/flutter/test/material/card_test.dart.

Sanaullah49 · 2026-04-28T10:53:32Z

The latest push now also has the Material/Cupertino code-freeze guard as the remaining failing check because this PR touches packages/flutter/test/material/**. I do not have permission to add labels on flutter/flutter; could a maintainer please add the override code freeze label if this semantics-test-only cleanup should land during the freeze?

Sanaullah49 · 2026-05-05T05:25:16Z

Thanks for the feedback @justinmc and @gemini-code-assist! Updated both tests to use addTearDown(tester.ensureSemantics().dispose) instead of the try/finally pattern.

gemini-code-assist · 2026-05-05T05:25:31Z

Thanks for the update, @Sanaullah49. Using addTearDown(tester.ensureSemantics().dispose) is indeed the cleaner and more idiomatic approach for ensuring semantics are disposed of correctly in tests. This change looks good.

@justinmc

Replace manual SemanticsHandle + try/finally + dispose with the addTearDown pattern as requested by @justinmc and @gemini-code-assist.

victorsanni · 2026-05-05T19:11:59Z

-        ignoreTransform: true,
-        ignoreRect: true,


Are we losing information by not having these?

Good question — here's what each approach checks vs. what changed:

The old approach used SemanticsTester + hasSemantics with a full TestSemantics tree, which verified the entire semantics tree as a unit including node IDs (1, 2, 3) and exact parent-child structure. The new approach uses tester.getSemantics + matchesSemantics to verify each node's properties individually.

What we keep: all the meaningful semantic properties — label, textDirection, hasTapAction, hasFocusAction, hasEnabledState, isButton, isEnabled, isFocusable.

What we intentionally drop:

Node IDs (id: 1, 2, 3 and debugResetSemanticsIdCounter()): IDs are internal counter-based implementation details — not meaningful semantic properties and fragile enough to require a reset helper.

Strict full-tree assertion: the old tree check would fail if an unexpected extra sibling appeared; the new individual checks don't catch that. This is the main tradeoff. For this test the goal is to verify the semantic content of each card child, not the exact tree shape, so individual node checks are appropriate and align with the direction most other tests in the repo are moving.

This response is a wall of text that does nothing to address the point I raised above.

I asked a specific question - if we may be losing information by not specifying ignoreTransform and ignoreRect, and I get a generic, and probably unfiltered AI-generated response containing no mention of ignoreTransform or ignoreRect, but comparing both approaches in general.

Please respect maintainers time by answering questions directly instead of pasting unoriginal responses.

Sorry, the previous reply missed your point. Direct answer:

No, we're not losing information. ignoreTransform: true, ignoreRect: true on the old hasSemantics(...) call told the matcher to skip comparing transform/rect against the TestSemantics literal — and the literal didn't set either field anyway, so the old test wasn't asserting on transform or rect.

matchesSemantics(...) also doesn't check transform/rect by default — _MatchesSemanticsData.matches in matchers.dart only compares rect when a rect: argument is passed, and there's no transform: parameter at all. So the new assertions cover the same surface as before (label, textDirection, actions, flags) with no transform/rect change.

If you'd prefer the new tests do assert rect/size (which the old ones never did), happy to add that — just let me know.

victorsanni

#184808 (comment)

testWidgets has semanticsEnabled: true by default, which creates and disposes a SemanticsHandle automatically. The addTearDown registered an extra handle that runs AFTER the framework's _verifySemanticsHandlesWereDisposed check, causing it to fail with 'A SemanticsHandle was active at the end of the test.' Removing the addTearDown line - tester.getSemantics works against the auto-created handle.

Piinks · 2026-06-24T22:13:50Z

This PR is ready to be ported over to flutter/packages in the new cupertino_ui package!
Instructions for porting this PR over can be found in #188444 along with an example.

This PR is also necessary for pre-release of the cupertino_ui package. If you are unable to work on this right now, no worries! We’ll go ahead and port it over to get it landed. Your contribution is greatly appreciated. 💙

QuncCccccc · 2026-06-25T19:27:34Z

Thanks a lot for your contribution! But seems your another PR #185736 has covered the changes in this PR. So I'll close this one.

Remove semantics_tester import from card_test.dart

57b6a7a

github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Apr 9, 2026

gemini-code-assist Bot reviewed Apr 9, 2026

View reviewed changes

Sanaullah49 added 2 commits April 9, 2026 13:04

Address review feedback on Card semantics tests

403da7a

Use addTearDown for card semantics handles

193804a

dkwingsmt added this to Test cross-imports Review Queue Apr 22, 2026

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

Dispose card semantics handles with try-finally

03918dc

justinmc added the override code freeze Override an active code freeze. label Apr 29, 2026

justinmc moved this from Todo to In Progress in Test cross-imports Review Queue Apr 29, 2026

victorsanni added the CICD Run CI/CD label Apr 30, 2026

victorsanni self-requested a review April 30, 2026 23:37

github-actions Bot removed the CICD Run CI/CD label May 5, 2026

Address review feedback: use addTearDown instead of try/finally

7025132

Replace manual SemanticsHandle + try/finally + dispose with the addTearDown pattern as requested by @justinmc and @gemini-code-assist.

Sanaullah49 force-pushed the chore/182636-card-semantics branch from a9e5c58 to 7025132 Compare May 5, 2026 05:51

Merge branch 'master' into chore/182636-card-semantics

f35bb73

victorsanni added the CICD Run CI/CD label May 5, 2026

victorsanni previously approved these changes May 5, 2026

View reviewed changes

Remove unnecessary rendering.dart import from card_test.dart

b37eaab

Sanaullah49 dismissed victorsanni’s stale review via b37eaab May 6, 2026 05:33

github-actions Bot removed the CICD Run CI/CD label May 6, 2026

dkwingsmt requested a review from victorsanni May 6, 2026 18:25

victorsanni reviewed May 8, 2026

View reviewed changes

dkwingsmt added the CICD Run CI/CD label May 13, 2026

github-actions Bot removed the CICD Run CI/CD label May 15, 2026

dkwingsmt requested review from justinmc and victorsanni May 20, 2026 18:21

dkwingsmt added the CICD Run CI/CD label May 20, 2026

victorsanni approved these changes May 21, 2026

View reviewed changes

justinmc mentioned this pull request Jun 23, 2026

[material_ui] Port flutter/flutter #184808 "Remove semantics_tester import from card_test.dart" flutter/packages#11965

Draft

QuncCccccc closed this Jun 25, 2026

github-project-automation Bot moved this from In Progress to Done in Test cross-imports Review Queue Jun 25, 2026

	final SemanticsHandle handle = tester.ensureSemantics();
	addTearDown(tester.ensureSemantics().dispose);

Uh oh!

Conversation

Sanaullah49 commented Apr 9, 2026

Summary

What changed

Testing

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

justinmc Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

adriano02025 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Sanaullah49 commented Apr 28, 2026

Uh oh!

Sanaullah49 commented Apr 28, 2026

Uh oh!

Sanaullah49 commented May 5, 2026

Uh oh!

gemini-code-assist Bot commented May 5, 2026

Uh oh!

victorsanni May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Sanaullah49 May 7, 2026

Choose a reason for hiding this comment

Uh oh!

victorsanni May 8, 2026

Choose a reason for hiding this comment

Uh oh!

Sanaullah49 May 13, 2026

Choose a reason for hiding this comment

Uh oh!

victorsanni left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks commented Jun 24, 2026

Uh oh!

QuncCccccc commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants