Remove semantics_tester cross-imports from material tests by Sanaullah49 · Pull Request #185736 · flutter/flutter

Sanaullah49 · 2026-04-29T10:24:24Z

Remove cross-directory test imports of semantics_tester.dart from material tests.

Duplicates packages/flutter/test/widgets/semantics_tester.dart verbatim into packages/flutter/test/material/, and rewires the four importing tests (card_test.dart, elevated_button_test.dart, material_button_test.dart, data_table_test.dart) to use the local copy.

This follows the duplication pattern used in the merged #184517 (live_text_utils), #184519 (editable_text_utils), and #184282 (navigator_utils).

Part of #182636.

Pre-launch Checklist

Duplicates `packages/flutter/test/widgets/semantics_tester.dart` into `packages/flutter/test/material/` so that material tests no longer reach into the widgets test directory for shared utilities. This unblocks the planned move of Material to flutter/packages. The local copy is verbatim. Four importing tests are rewired to use it: card_test.dart, elevated_button_test.dart, material_button_test.dart, and data_table_test.dart. Part of flutter#182636.

gemini-code-assist

Code Review

This pull request moves semantics_tester.dart to the material test directory and updates corresponding imports. Feedback identifies several logic errors in the new file, including a tautology in the controlsNodes validation, flawed locale comparison logic, and a syntax error in an assertion within the _IncludesNodeWith constructor. Additionally, an error message for text selection mismatches was found to be inconsistent.

gemini-code-assist · 2026-04-29T10:26:09Z

+    if (textSelection?.baseOffset != nodeData.textSelection?.baseOffset ||
+        textSelection?.extentOffset != nodeData.textSelection?.extentOffset) {
+      return fail(
+        'expected node id $id to have textSelection [${textSelection?.baseOffset}, ${textSelection?.end}] but found: [${nodeData.textSelection?.baseOffset}, ${nodeData.textSelection?.extentOffset}].',


The error message is inconsistent. It uses textSelection?.end for the expected value but nodeData.textSelection?.extentOffset for the actual value. Since the comparison on lines 461-462 uses baseOffset and extentOffset, the error message should consistently use these properties to accurately reflect why the match failed.

Suggested change

'expected node id $id to have textSelection [${textSelection?.baseOffset}, ${textSelection?.end}] but found: [${nodeData.textSelection?.baseOffset}, ${nodeData.textSelection?.extentOffset}].',

'expected node id $id to have textSelection [${textSelection?.baseOffset}, ${textSelection?.extentOffset}] but found: [${nodeData.textSelection?.baseOffset}, ${nodeData.textSelection?.extentOffset}].',

gemini-code-assist · 2026-04-29T10:26:10Z

+      );
+    }
+
+    if (controlsNodes != controlsNodes && !setEquals(controlsNodes, node.controlsNodes)) {


The condition controlsNodes != controlsNodes is a tautology that always evaluates to false, which means the setEquals check and the subsequent failure message will never be reached. This effectively disables the validation for controlsNodes.

Suggested change

if (controlsNodes != controlsNodes && !setEquals(controlsNodes, node.controlsNodes)) {

if (controlsNodes != null && !setEquals(controlsNodes, node.controlsNodes)) {

Added to #185900

gemini-code-assist · 2026-04-29T10:26:10Z

+              (second[i] as LocaleStringAttribute).locale !=
+                  (second[i] as LocaleStringAttribute).locale)) {


The locale comparison logic is flawed as it compares second[i].locale to itself. It should compare the locale from first[i] with the locale from second[i].

Suggested change

(second[i] as LocaleStringAttribute).locale !=

(second[i] as LocaleStringAttribute).locale)) {

(first[i] as LocaleStringAttribute).locale !=

(second[i] as LocaleStringAttribute).locale)) {

Good bot, this looks like a bug indeed. Probably not something to fix in this PR though since it occurs in multiple places now. I've created an issue: #185900

gemini-code-assist · 2026-04-29T10:26:10Z

+             inputType != null,
+         minValue != null || maxValue != null,


The assert statement in the _IncludesNodeWith constructor incorrectly uses a comma after inputType != null, making the subsequent expression (minValue != null || maxValue != null) the second argument to assert (the error message) rather than part of the boolean condition. This causes the assertion to fail if only minValue or maxValue are provided.

inputType != null || minValue != null || maxValue != null,

Yup added to #185900

Sanaullah49 · 2026-04-29T10:27:46Z

The Check Code Freeze failure looks expected — this PR touches packages/flutter/test/material/ and the Material/Cupertino freeze (#184093) is in effect.

Requesting the override: code freeze label: this PR directly facilitates the flutter/packages migration that the freeze is intended to enable. From the umbrella issue #182636: "Some test files import test utilities from other library's tests. This won't be possible once Material and Cupertino are moved to the flutter/packages repo." Removing these cross-imports is the prerequisite for the move, not a behavioral change to Material.

The four other PRs in this cluster — #184517, #184519, #184282, #184193 — were all merged before the freeze cutoff. The remaining cross-import work (this PR + #185250 and others) needs the override or it can't land before the migration window opens.

cc @justinmc (umbrella issue owner)

justinmc

LGTM 👍

justinmc · 2026-05-01T16:43:21Z

+              (second[i] as LocaleStringAttribute).locale !=
+                  (second[i] as LocaleStringAttribute).locale)) {


Good bot, this looks like a bug indeed. Probably not something to fix in this PR though since it occurs in multiple places now. I've created an issue: #185900

justinmc · 2026-05-01T16:49:36Z

+             inputType != null,
+         minValue != null || maxValue != null,


Yup added to #185900

justinmc · 2026-05-01T16:49:51Z

+      );
+    }
+
+    if (controlsNodes != controlsNodes && !setEquals(controlsNodes, node.controlsNodes)) {


Added to #185900

justinmc · 2026-05-01T16:51:08Z

I agree about the code freeze override, added. Thanks again for your work on these!

Sanaullah49 · 2026-05-05T05:14:19Z

Hi @justinmc — CI is now all green after a rerun of the flaky Windows framework_tests_widgets shard. Could you please merge or add the autosubmit label when you get a chance? Thanks!

auto-submit · 2026-05-08T22:27:53Z

autosubmit label was removed for flutter/flutter/185736, because - The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.

Sanaullah49 · 2026-05-13T06:07:49Z

Hi @justinmc @victorsanni — CI is green again (Linux analyze passed on the latest run, mergeStateStatus is CLEAN). Could you re-add the autosubmit label when you have a moment? Thanks!

Three bugs in TestSemantics that caused assertions/comparisons to never work correctly: 1. `controlsNodes != controlsNodes` always evaluates to false (self- comparison); fixed to `controlsNodes != null` so the setEquals check actually runs. 2. Locale comparison compared `second[i]` to itself instead of to `first[i]`, making the check a no-op. 3. A stray comma after `inputType != null` split the assert condition, making `minValue != null || maxValue != null` the error message instead of part of the condition. All three bugs existed identically in both test/widgets/semantics_tester.dart and test/material/semantics_tester.dart (the latter is a copy introduced in flutter#185736). Fixes flutter#185900

Sanaullah49 · 2026-05-21T10:09:48Z

Linux analyzer_benchmark failed before the benchmark ran: flutter config --clear-features hit curl: (56) OpenSSL SSL_read: error:0A000126:SSL routines::unexpected eof while reading while downloading the Dart SDK. The rest of CI is green. Could someone rerun that shard when convenient?

justinmc

LGTM 👍

victorsanni

I did a grep of import '../widgets/semantics_tester.dart'; and found many more instances, why are there only 2 in this PR?

Piinks · 2026-06-24T22:16:04Z

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. 💙

github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) labels Apr 29, 2026

gemini-code-assist Bot reviewed Apr 29, 2026

View reviewed changes

Sanaullah49 force-pushed the remove-semantics-tester-cross-imports branch from 4a682d5 to 7a7c1a3 Compare April 29, 2026 10:38

justinmc added this to Test cross-imports Review Queue May 1, 2026

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

justinmc mentioned this pull request May 1, 2026

TestSemantics typo bugs #185900

Open

justinmc previously approved these changes May 1, 2026

View reviewed changes

justinmc added CICD Run CI/CD override code freeze Override an active code freeze. labels May 1, 2026

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

Delis2001 mentioned this pull request May 1, 2026

Fix TestSemantics typo bugs Delis2001/flutter#1

Open

Rerun CI: trigger retry of flaky Windows framework_tests_widgets

46a3440

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

sjhddh mentioned this pull request May 5, 2026

fix(test): correct three TestSemantics typo bugs in semantics_tester.dart #186058

Closed

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

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

Merge branch 'master' into remove-semantics-tester-cross-imports

ec93cd0

github-actions Bot added CICD Run CI/CD and removed CICD Run CI/CD a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) labels May 8, 2026

victorsanni previously approved these changes May 8, 2026

View reviewed changes

victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label May 8, 2026

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 8, 2026

This was referenced May 10, 2026

Fix TestSemantics typo bugs in semantics_tester (widgets and material) #186313

Closed

Fix TestSemantics typo bugs in semantics_tester (widgets and material) #186314

Closed

Apply dart format to elevated_button_test.dart

17773e7

Sanaullah49 dismissed stale reviews from victorsanni and justinmc via 17773e7 May 13, 2026 06:29

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

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

BilalRehman08 mentioned this pull request May 17, 2026

Fix TestSemantics typo bugs in semantics_tester (widgets and material) #186641

Closed

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

victorsanni previously approved these changes May 21, 2026

View reviewed changes

Merge branch 'master' into remove-semantics-tester-cross-imports

b10a19c

victorsanni added CICD Run CI/CD and removed CICD Run CI/CD labels May 21, 2026

Merge branch 'master' into remove-semantics-tester-cross-imports

06ac795

dkwingsmt dismissed victorsanni’s stale review via 06ac795 May 27, 2026 18:03

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

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

justinmc approved these changes Jun 4, 2026

View reviewed changes

dkwingsmt requested a review from victorsanni June 10, 2026 18:11

victorsanni reviewed Jun 17, 2026

View reviewed changes

This was referenced Jun 25, 2026

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

Open

Remove semantics_tester import from card_test.dart #184808

Closed

	'expected node id $id to have textSelection [${textSelection?.baseOffset}, ${textSelection?.end}] but found: [${nodeData.textSelection?.baseOffset}, ${nodeData.textSelection?.extentOffset}].',
	'expected node id $id to have textSelection [${textSelection?.baseOffset}, ${textSelection?.extentOffset}] but found: [${nodeData.textSelection?.baseOffset}, ${nodeData.textSelection?.extentOffset}].',

	if (controlsNodes != controlsNodes && !setEquals(controlsNodes, node.controlsNodes)) {
	if (controlsNodes != null && !setEquals(controlsNodes, node.controlsNodes)) {

		(second[i] as LocaleStringAttribute).locale !=
		(second[i] as LocaleStringAttribute).locale)) {

Uh oh!

Conversation

Sanaullah49 commented Apr 29, 2026

Pre-launch Checklist

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 29, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

justinmc May 1, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

justinmc May 1, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

justinmc May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Sanaullah49 commented Apr 29, 2026

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc May 1, 2026

Choose a reason for hiding this comment

Uh oh!

justinmc May 1, 2026

Choose a reason for hiding this comment

Uh oh!

justinmc May 1, 2026

Choose a reason for hiding this comment

Uh oh!

justinmc commented May 1, 2026

Uh oh!

Sanaullah49 commented May 5, 2026

Uh oh!

auto-submit Bot commented May 8, 2026

Uh oh!

Sanaullah49 commented May 13, 2026

Uh oh!

Sanaullah49 commented May 21, 2026

Uh oh!

justinmc left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants