Remove no shuffle from flutter_driver extension_test.dart and mock flutter.process channel by ValentinVignal · Pull Request #187559 · flutter/flutter

ValentinVignal · 2026-06-04T09:46:10Z

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

gemini-code-assist

Code Review

This pull request removes the @Tags(<String>['no-shuffle']) tag, the library; declaration, and the associated TODO comment from packages/flutter_driver/test/src/real_tests/extension_test.dart. There are no review comments, and I have no feedback to provide.

ValentinVignal · 2026-06-06T04:47:51Z

This needs a bit more work

ValentinVignal · 2026-06-22T11:32:54Z

+  TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler(
+    SystemChannels.platform,
+    (MethodCall methodCall) async {
+      return null;
+    },


I'm not too sure where this should be added, any advice on that?

Yeah I think this should be reverted and moved elsewhere. Instead, maybe register SystemChannels.platform mock handling directly within extension_test.dart (e.g. in a top-level setUp or group setUp), where text widgets are constructed and tested.

Sure, this is done in Clean test

gemini-code-assist

Code Review

This pull request removes the 'no-shuffle' tag from extension_test.dart and adds teardown logic to clean up mock message handlers and dispose of text controllers. It also mocks SystemChannels.platform inside mockFlutterAssets(). The review feedback suggests that mocking SystemChannels.platform inside mockFlutterAssets violates the single responsibility principle and may cause side effects. Additionally, the feedback recommends refactoring duplicated mock setup and teardown logic for SystemChannels.processText and hello channels into helper functions to improve maintainability.

gemini-code-assist · 2026-06-22T11:33:44Z

+  TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler(
+    SystemChannels.platform,
+    (MethodCall methodCall) async {
+      return null;
+    },
+  );


Mocking SystemChannels.platform inside mockFlutterAssets() violates the single responsibility principle, as this function is specifically intended for mocking assets. Additionally, setting a global mock handler for SystemChannels.platform here affects all tests and might interfere with tests that expect to handle or mock platform messages differently. Consider moving this mock setup to a more appropriate global initialization phase or binding setup, or keep it localized to the tests that actually require it.

References

Optimize for readability and maintain clean code structure. ^(link)

gemini-code-assist · 2026-06-22T11:33:44Z

+    tester.binding.defaultBinaryMessenger.setMockMethodCallHandler(
+      SystemChannels.processText,
+      (_) async => null,
+    );
+    addTearDown(() {
+      tester.binding.defaultBinaryMessenger.setMockMethodCallHandler(
+        SystemChannels.processText,
+        null,
+      );
+    });


The mocking and teardown of SystemChannels.processText is duplicated across multiple tests in this file (e.g., in getText, enableTextEntryEmulation false, enableTextEntryEmulation true, and press done trigger onSubmitted and change value). To improve maintainability and reduce boilerplate, consider extracting this into a helper function at the file level, such as void _mockProcessText(WidgetTester tester) { ... }.

References

Suggest simplification and refactoring to enhance readability and maintainability. ^(link)

gemini-code-assist · 2026-06-22T11:33:45Z

+        addTearDown(() {
+          tester.binding.defaultBinaryMessenger.setMockMessageHandler('helloChannel1', null);
+          tester.binding.defaultBinaryMessenger.setMockMessageHandler('helloChannel2', null);
+        });


The teardown logic to clear helloChannel1 and helloChannel2 is duplicated across multiple tests (e.g., lines 354-357, 411-414, and 472-475). Consider extracting this into a helper function to keep the tests clean and maintainable.

References

Suggest simplification and refactoring to enhance readability and maintainability. ^(link)

Piinks · 2026-07-20T23:41:12Z

+  TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler(
+    SystemChannels.platform,
+    (MethodCall methodCall) async {
+      return null;
+    },


Yeah I think this should be reverted and moved elsewhere. Instead, maybe register SystemChannels.platform mock handling directly within extension_test.dart (e.g. in a top-level setUp or group setUp), where text widgets are constructed and tested.

Piinks · 2026-07-20T23:43:04Z

                    key: const ValueKey<String>('text3'),
-                    controller: TextEditingController(text: 'Hello3'),
+                    controller: controller3,
                    focusNode: FocusNode(),


While looking at this, I think this FocusNode leaks. Can you declare final focusNode3 = FocusNode(); before pumpWidget and register addTearDown(focusNode3.dispose); to resolve?

Good point, I did it in Clean test

Piinks

LGTM thank you!

…12325) Manual roll requested by [email protected] flutter/flutter@c83f80b...2a230d1 2026-07-29 [email protected] Clarify CustomScrollView use cases (flutter/flutter#189692) 2026-07-29 [email protected] Fix TreeSliver first node clipping during expand/collapse animation (flutter/flutter#188626) 2026-07-29 [email protected] iOS: Migrate VSyncClient tests to Swift Testing (flutter/flutter#190054) 2026-07-29 [email protected] Remove no shuffle from flutter_driver extension_test.dart and mock flutter.process channel (flutter/flutter#187559) 2026-07-29 [email protected] Adds missing await on `instantiateImageCodecFromBuffer` (flutter/flutter#188910) 2026-07-29 [email protected] [web] Provide Content-Length header for CanvasKit files in flutter test (flutter/flutter#190155) 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 Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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

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

github-actions Bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Jun 4, 2026

gemini-code-assist Bot reviewed Jun 4, 2026

View reviewed changes

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

ValentinVignal marked this pull request as draft June 6, 2026 04:47

ValentinVignal force-pushed the flutter-driver/remove-no-shuffle-from-extension-test branch from f2199fb to 9cb6f9a Compare June 22, 2026 11:31

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

ValentinVignal changed the title ~~Remove no shuffle from flutter_driver extension_test.dart~~ Remove no shuffle from flutter_driver extension_test.dart and mock flutter.process channel Jun 22, 2026

ValentinVignal marked this pull request as ready for review June 22, 2026 11:32

ValentinVignal commented Jun 22, 2026

View reviewed changes

gemini-code-assist Bot reviewed Jun 22, 2026

View reviewed changes

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

Piinks self-requested a review June 23, 2026 22:15

ValentinVignal force-pushed the flutter-driver/remove-no-shuffle-from-extension-test branch 2 times, most recently from c1e5b56 to b890ae2 Compare July 9, 2026 08:53

Piinks reviewed Jul 20, 2026

View reviewed changes

ValentinVignal force-pushed the flutter-driver/remove-no-shuffle-from-extension-test branch from b890ae2 to f51a044 Compare July 28, 2026 10:17

ValentinVignal requested a review from Piinks July 28, 2026 10:18

Piinks approved these changes Jul 28, 2026

View reviewed changes

ValentinVignal added 6 commits July 28, 2026 18:09

Remove no shuffle from extension_test.dart

d36f178

fix tests

d382956

Dispose controller and mock process text

3cabcd7

Mock platform channel

afbd134

Remove bad files

96e86e2

Clean test

2cbf4eb

Piinks force-pushed the flutter-driver/remove-no-shuffle-from-extension-test branch from f51a044 to 2cbf4eb Compare July 28, 2026 23:09

Piinks added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Jul 28, 2026

auto-submit Bot added this pull request to the merge queue Jul 29, 2026

Merged via the queue into flutter:master with commit e26a93d Jul 29, 2026
28 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 29, 2026

Conversation

ValentinVignal commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

ValentinVignal commented Jun 6, 2026

Uh oh!

ValentinVignal Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

ValentinVignal Jul 28, 2026

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

ValentinVignal Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ValentinVignal commented Jun 4, 2026 •

edited

Loading