fix: resolve TestSemantics comparison and assert issues by harshyadavDeveloper · Pull Request #185921 · flutter/flutter

harshyadavDeveloper · 2026-05-02T06:46:09Z

Description

Fixes multiple issues in TestSemantics where incorrect comparisons
and an invalid assert condition resulted in unintended behavior.

Changes include:

Corrected comparison of LocaleStringAttribute.locale to avoid
comparing a value with itself
Fixed incorrect comparison of controlsNodes with itself
Updated assert statement to properly combine conditions using &&
instead of passing multiple arguments

These fixes improve correctness and prevent logical errors in
semantics testing.

Issues fixed by this PR

Fixes #185900

Pre-launch Checklist

flutter-dashboard · 2026-05-02T06:46:12Z

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 you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

gemini-code-assist

Code Review

This pull request corrects tautological comparisons in semantics_tester.dart and modifies an assert statement in the _IncludesNodeWith matcher. Feedback identifies a typo and redundant check in the controlsNodes logic, and highlights a regression in the assert condition that incorrectly requires both property and range values, alongside a violation of the dart format style requirement.

gemini-code-assist · 2026-05-02T06:47:49Z

    }

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


There is a typo in the property name: it should be node.controlsNodes (plural), not node.controlsNode. Additionally, the identity check controlsNodes != node.controlsNodes is redundant because setEquals already performs an identity check internally and handles null values correctly.

Suggested change

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

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

gemini-code-assist · 2026-05-02T06:47:49Z

+  (label != null ||
+      value != null ||
+      actions != null ||
+      flags != null ||
+      flagsCollection != null ||
+      tags != null ||
+      increasedValue != null ||
+      decreasedValue != null ||
+      scrollPosition != null ||
+      scrollExtentMax != null ||
+      scrollExtentMin != null ||
+      maxValueLength != null ||
+      currentValueLength != null ||
+      inputType != null) &&
+  (minValue != null || maxValue != null),
+);


The logic in the assert statement has been changed from "at least one property" to "at least one property from the first group AND one of the min/max values". This is a regression that will break tests matching only on labels or values. It should use || to ensure at least one of any property is provided. Also, the formatting should follow dart format as per the repository style guide.

Suggested change

(label != null ||

value != null ||

actions != null ||

flags != null ||

flagsCollection != null ||

tags != null ||

increasedValue != null ||

decreasedValue != null ||

scrollPosition != null ||

scrollExtentMax != null ||

scrollExtentMin != null ||

maxValueLength != null ||

currentValueLength != null ||

inputType != null) &&

(minValue != null || maxValue != null),

);

label != null ||

value != null ||

actions != null ||

flags != null ||

flagsCollection != null ||

tags != null ||

increasedValue != null ||

decreasedValue != null ||

scrollPosition != null ||

scrollExtentMax != null ||

scrollExtentMin != null ||

maxValueLength != null ||

currentValueLength != null ||

inputType != null ||

minValue != null ||

maxValue != null,

);

References

All Dart code is formatted using dart format. This is enforced by CI. ^(link)

The gemini bots suggestion sounds valid

harshyadavDeveloper · 2026-05-02T07:21:26Z

Added tests to cover the TestSemantics fixes. Verified that the updated tests pass locally.

Istiak-Ahmed78 · 2026-05-04T18:27:31Z

    );
    semantics.dispose();
  });
+


Hi @harshyadavDeveloper , your tests require fixes. the tests you wrote honestly does not test the comparison errors we are addressing.

Istiak-Ahmed78 · 2026-05-04T18:39:53Z

+    locale: const Locale('fr'),
+  );
+
+  expect(first, isNot(equals(second)));


Your tests compares two TestSemantics objects using equals(), but the bugs are in _stringAttributesEqual() & _matches() of the TestSemantics class.
Let me tell me tell you what you are doing:

Creating two TestSemantics objects

Comparing them using equals()

Checks they're NOT equal

This test only compare two TestSemantics objects. Don't test the actual bugges. equals(second) calls the == operator. But TestSemantics does NOT override ==, So it uses the default Dart ==, Test PASSES

Istiak-Ahmed78 · 2026-05-04T18:42:50Z

+    controlsNodes: <String>{'3', '4'},
+  );
+
+  expect(first, isNot(equals(second)));


Same happens here!!

Istiak-Ahmed78 · 2026-05-04T19:54:20Z

+  testWidgets('TestSemantics detects different locales correctly', (WidgetTester tester) async {
+  final first = TestSemantics(
+    id: 1,
+    label: 'Hello',
+    locale: const Locale('en'),
+  );
+
+  final second = TestSemantics(
+    id: 1,
+    label: 'Hello',
+    locale: const Locale('fr'),
+  );
+
+  expect(first, isNot(equals(second)));
+});
+
+  testWidgets('TestSemantics detects different controlNodes correctly', (WidgetTester tester) async {
+  final first = TestSemantics(
+    id: 1,
+    controlsNodes: <String>{'1', '2'},
+  );
+
+  final second = TestSemantics(
+    id: 1,
+    controlsNodes: <String>{'3', '4'},
+  );
+
+  expect(first, isNot(equals(second)));
+});


For verification of the concept. I ran your test code in my IDE with bugs in semantics_tester.dart. All test PASSED!! However, I'm providing you correct test code. I ensured all these tests fails with buggy behaviour and passes all when the issues are fixed.

Suggested change

testWidgets('TestSemantics detects different locales correctly', (WidgetTester tester) async {

final first = TestSemantics(

id: 1,

label: 'Hello',

locale: const Locale('en'),

);

final second = TestSemantics(

id: 1,

label: 'Hello',

locale: const Locale('fr'),

);

expect(first, isNot(equals(second)));

});

testWidgets('TestSemantics detects different controlNodes correctly', (WidgetTester tester) async {

final first = TestSemantics(

id: 1,

controlsNodes: <String>{'1', '2'},

);

final second = TestSemantics(

id: 1,

controlsNodes: <String>{'3', '4'},

);

expect(first, isNot(equals(second)));

});

testWidgets('hasSemantics should detect controlsNodes mismatch', (WidgetTester tester) async {

final semantics = SemanticsTester(tester);

await tester.pumpWidget(

Semantics(label: 'Test', textDirection: TextDirection.ltr, child: Container()),

);

// Test expects controlsNodes but widget doesn't have any

final testSemantics = TestSemantics.root(

children: <TestSemantics>[

TestSemantics.rootChild(

id: 1,

label: 'Test',

rect: TestSemantics.fullScreen,

controlsNodes: <String>{'node1'},

),

],

);

expect(semantics, isNot(hasSemantics(testSemantics, ignoreId: true)));

semantics.dispose();

});

test('includesNodeWith accepts minValue only', () {

expect(() => includesNodeWith(minValue: '0'), returnsNormally);

});

test('includesNodeWith accepts maxValue only', () {

expect(() => includesNodeWith(maxValue: '100'), returnsNormally);

});

test('includesNodeWith rejects no properties', () {

expect(() => includesNodeWith(), throwsAssertionError);

});

testWidgets('nodesWith detects locale attribute mismatch', (WidgetTester tester) async {

final semantics = SemanticsTester(tester);

await tester.pumpWidget(

const Text.rich(

TextSpan(text: 'Hello', locale: Locale('en')),

textDirection: TextDirection.ltr,

),

);

final attributedLabel = AttributedString(

'Hello',

attributes: [

LocaleStringAttribute(range: const TextRange(start: 0, end: 5), locale: const Locale('fr')),

],

);

final Iterable<SemanticsNode> nodes = semantics.nodesWith(attributedLabel: attributedLabel);

expect(nodes, isEmpty);

semantics.dispose();

});

justinmc

Thanks for the PR! +1 to @Istiak-Ahmed78's points here.

chunhtai · 2026-05-27T21:44:20Z

closing this pr in favor of #185975

fix: resolve TestSemantics comparison and assert issues

8e902a7

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

gemini-code-assist Bot reviewed May 2, 2026

View reviewed changes

test: add coverage for TestSemantics comparison fixes

527a832

Merge branch 'master' into fix-test-semantics-bugs

943533a

This was referenced May 4, 2026

Fix self-comparison and assert typos in TestSemantics #185975

Open

[framework] Fix TestSemantics self-comparison bugs and stray comma #185968

Closed

Istiak-Ahmed78 reviewed May 4, 2026

View reviewed changes

justinmc mentioned this pull request May 4, 2026

TestSemantics typo bugs #185900

Open

justinmc reviewed May 4, 2026

View reviewed changes

This was referenced May 8, 2026

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

Closed

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

Closed

chunhtai self-requested a review May 13, 2026 20:49

chunhtai closed this May 27, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: resolve TestSemantics comparison and assert issues#185921

fix: resolve TestSemantics comparison and assert issues#185921
harshyadavDeveloper wants to merge 3 commits into
flutter:masterfrom
harshyadavDeveloper:fix-test-semantics-bugs

harshyadavDeveloper commented May 2, 2026 •

edited

Loading

Uh oh!

flutter-dashboard Bot commented May 2, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Uh oh!

gemini-code-assist Bot May 2, 2026

Uh oh!

Istiak-Ahmed78 May 4, 2026 •

edited

Loading

Uh oh!

harshyadavDeveloper commented May 2, 2026

Uh oh!

Istiak-Ahmed78 May 4, 2026 •

edited

Loading

Uh oh!

Istiak-Ahmed78 May 4, 2026 •

edited

Loading

Uh oh!

Istiak-Ahmed78 May 4, 2026

Uh oh!

Istiak-Ahmed78 May 4, 2026

Uh oh!

justinmc left a comment

Uh oh!

chunhtai commented May 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

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

-  testWidgets('TestSemantics detects different locales correctly', (WidgetTester tester) async {
-  final first = TestSemantics(
-    id: 1,
-    label: 'Hello',
-    locale: const Locale('en'),
-  );
-  final second = TestSemantics(
-    id: 1,
-    label: 'Hello',
-    locale: const Locale('fr'),
-  );
-  expect(first, isNot(equals(second)));
-});
-  testWidgets('TestSemantics detects different controlNodes correctly', (WidgetTester tester) async {
-  final first = TestSemantics(
-    id: 1,
-    controlsNodes: <String>{'1', '2'},
-  );
-  final second = TestSemantics(
-    id: 1,
-    controlsNodes: <String>{'3', '4'},
-  );
-  expect(first, isNot(equals(second)));
-});
+  testWidgets('hasSemantics should detect controlsNodes mismatch', (WidgetTester tester) async {
+    final semantics = SemanticsTester(tester);
+    await tester.pumpWidget(
+      Semantics(label: 'Test', textDirection: TextDirection.ltr, child: Container()),
+    );
+    // Test expects controlsNodes but widget doesn't have any
+    final testSemantics = TestSemantics.root(
+      children: <TestSemantics>[
+        TestSemantics.rootChild(
+          id: 1,
+          label: 'Test',
+          rect: TestSemantics.fullScreen,
+          controlsNodes: <String>{'node1'},
+        ),
+      ],
+    );
+    expect(semantics, isNot(hasSemantics(testSemantics, ignoreId: true)));
+    semantics.dispose();
+  });
+  test('includesNodeWith accepts minValue only', () {
+    expect(() => includesNodeWith(minValue: '0'), returnsNormally);
+  });
+  test('includesNodeWith accepts maxValue only', () {
+    expect(() => includesNodeWith(maxValue: '100'), returnsNormally);
+  });
+  test('includesNodeWith rejects no properties', () {
+    expect(() => includesNodeWith(), throwsAssertionError);
+  });
+    testWidgets('nodesWith detects locale attribute mismatch', (WidgetTester tester) async {
+    final semantics = SemanticsTester(tester);
+    await tester.pumpWidget(
+      const Text.rich(
+        TextSpan(text: 'Hello', locale: Locale('en')),
+        textDirection: TextDirection.ltr,
+      ),
+    );
+    final attributedLabel = AttributedString(
+      'Hello',
+      attributes: [
+        LocaleStringAttribute(range: const TextRange(start: 0, end: 5), locale: const Locale('fr')),
+      ],
+    );
+    final Iterable<SemanticsNode> nodes = semantics.nodesWith(attributedLabel: attributedLabel);
+    expect(nodes, isEmpty);
+    semantics.dispose();
+  });

Uh oh!

Conversation

harshyadavDeveloper commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues fixed by this PR

Pre-launch Checklist

Uh oh!

flutter-dashboard Bot commented May 2, 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 May 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

Istiak-Ahmed78 May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harshyadavDeveloper commented May 2, 2026

Uh oh!

Istiak-Ahmed78 May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Istiak-Ahmed78 May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Istiak-Ahmed78 May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Istiak-Ahmed78 May 4, 2026

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

chunhtai commented May 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

harshyadavDeveloper commented May 2, 2026 •

edited

Loading

Istiak-Ahmed78 May 4, 2026 •

edited

Loading

Istiak-Ahmed78 May 4, 2026 •

edited

Loading

Istiak-Ahmed78 May 4, 2026 •

edited

Loading