Skip to content

Conversation

@dkwingsmt
Copy link
Contributor

When #176519 was authored, there were no CI shards that run flutter_test with leak tests, and therefore the test had to be placed in a separate file. Now that #176992 has added misc leak tracking, the test case can be placed in the same file as other tests for WidgetTester.

Pre-launch Checklist

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

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.

@github-actions 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 Jan 6, 2026
@dkwingsmt dkwingsmt requested a review from victorsanni January 6, 2026 20:32
Copy link
Contributor

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

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 refactors the test suite by merging widget_tester_leaks_free_test.dart into widget_tester_test.dart. This is a good cleanup, consolidating tests for WidgetTester now that the CI supports leak tracking in this shard. I've added one suggestion to refactor the newly added test to reduce code duplication and improve maintainability.

Comment on lines +822 to +834
var counterByWidgets = 0;
final RestorationManager managerByWidgets = WidgetsBinding.instance.restorationManager;
expect(managerByWidgets, isA<TestRestorationManager>());
managerByWidgets.addListener(() => counterByWidgets++);
managerByWidgets.notifyListeners();
expect(counterByWidgets, 1);

var counterByServices = 0;
final RestorationManager managerByServices = ServicesBinding.instance.restorationManager;
expect(managerByServices, isA<TestRestorationManager>());
managerByServices.addListener(() => counterByServices++);
managerByServices.notifyListeners();
expect(counterByServices, 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

To improve maintainability and reduce code duplication, you could extract the repeated logic for testing a RestorationManager into a local helper function. This aligns with the style guide's principles of optimizing for readability and avoiding duplication.

Suggested change
var counterByWidgets = 0;
final RestorationManager managerByWidgets = WidgetsBinding.instance.restorationManager;
expect(managerByWidgets, isA<TestRestorationManager>());
managerByWidgets.addListener(() => counterByWidgets++);
managerByWidgets.notifyListeners();
expect(counterByWidgets, 1);
var counterByServices = 0;
final RestorationManager managerByServices = ServicesBinding.instance.restorationManager;
expect(managerByServices, isA<TestRestorationManager>());
managerByServices.addListener(() => counterByServices++);
managerByServices.notifyListeners();
expect(counterByServices, 1);
void testManager(RestorationManager manager) {
var counter = 0;
expect(manager, isA<TestRestorationManager>());
manager.addListener(() => counter++);
manager.notifyListeners();
expect(counter, 1);
}
testManager(WidgetsBinding.instance.restorationManager);
testManager(ServicesBinding.instance.restorationManager);
References
  1. The Flutter style guide emphasizes optimizing for readability and avoiding duplication. This suggestion improves both by refactoring repeated code into a helper function. (link)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I slightly prefer the current version for clarity over conciseness.

@victorsanni victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 9, 2026
@auto-submit auto-submit bot added this pull request to the merge queue Jan 9, 2026
Merged via the queue into flutter:master with commit cfb3f18 Jan 10, 2026
69 of 70 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 10, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 10, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 10, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 10, 2026
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jan 10, 2026
Roll Flutter from 01d37bc to 73769a2 (65 revisions)

flutter/flutter@01d37bc...73769a2

2026-01-10 engine-flutter-autoroll@skia.org Roll Dart SDK from 5e855c2bb3ef to 87fbfd5381b6 (1 revision) (flutter/flutter#180800)
2026-01-10 engine-flutter-autoroll@skia.org Roll Skia from b2b109f0e980 to f39cc645b1dd (2 revisions) (flutter/flutter#180796)
2026-01-10 engine-flutter-autoroll@skia.org Roll Dart SDK from b7963905e6a2 to 5e855c2bb3ef (2 revisions) (flutter/flutter#180794)
2026-01-10 ahmedsameha1@gmail.com Make sure that a CupertinoTabScaffold doesn't crash in 0x0 environment (flutter/flutter#179824)
2026-01-10 engine-flutter-autoroll@skia.org Roll Dart SDK from d25ad331b7ea to b7963905e6a2 (2 revisions) (flutter/flutter#180783)
2026-01-10 ahmedsameha1@gmail.com Make sure that a Container doesn't crash in 0x0 environment (flutter/flutter#180350)
2026-01-10 ahmedsameha1@gmail.com Make sure that an Expansible doesn't crash in 0x0 environment (flutter/flutter#180478)
2026-01-10 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Enabled some disabled impeller fragment shader dart tests (#180759)" (flutter/flutter#180785)
2026-01-09 dkwingsmt@users.noreply.github.com Merge `widget_tester_leaks_free_test.dart` into `widget_tester_test.dart` (flutter/flutter#180600)
2026-01-09 codefu@google.com fix: there are no riscv fuchsia artifacts (flutter/flutter#180779)
2026-01-09 engine-flutter-autoroll@skia.org Roll Skia from 7386219151e6 to b2b109f0e980 (1 revision) (flutter/flutter#180771)
2026-01-09 chinmaygarde@google.com Re-prioritize pipeline compile jobs and perform them eagerly instead of waiting. (flutter/flutter#180022)
2026-01-09 30870216+gaaclarke@users.noreply.github.com Enabled some disabled impeller fragment shader dart tests (flutter/flutter#180759)
2026-01-09 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from rxeg-6UB678HKJ4UQ... to 83Favz_zzMzdVuOHg... (flutter/flutter#180765)
2026-01-09 jhy03261997@gmail.com [A11y ] Add `clearSemantics`in table (flutter/flutter#180665)
2026-01-09 chinmaygarde@google.com Update CODEOWNERS to remove chinmaygarde. (flutter/flutter#180703)
2026-01-09 vhaudiquet343@hotmail.fr [ Tool ] Add support for linux riscv64 architecture (flutter/flutter#178711)
2026-01-09 engine-flutter-autoroll@skia.org Roll Skia from e9b3264ade0c to 7386219151e6 (12 revisions) (flutter/flutter#180754)
2026-01-09 engine-flutter-autoroll@skia.org Roll Dart SDK from fe2ba2c5dd50 to d25ad331b7ea (10 revisions) (flutter/flutter#180741)
2026-01-09 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Unpin google_mobile_ads (#180573)" (flutter/flutter#180761)
2026-01-09 koichi20021217@gmail.com Fix typo in dropdown_menu.dart (flutter/flutter#180172)
2026-01-09 engine-flutter-autoroll@skia.org Roll Packages from 039a026 to 51fe1d9 (1 revision) (flutter/flutter#180742)
2026-01-09 goderbauer@google.com Unpin google_mobile_ads (flutter/flutter#180573)
2026-01-09 evanwall@buffalo.edu Update flutter changelog for 3.38.6 (flutter/flutter#180708)
2026-01-08 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Fix iOS xattr removal to clear all extended attributes (#180355)" (flutter/flutter#180709)
2026-01-08 ahmedsameha1@gmail.com Make sure that an EditableText doesn't crash in 0x0 environment (flutter/flutter#180457)
2026-01-08 matt.kosarek@canonical.com Implementation of tooltip windows for win32 (flutter/flutter#179147)
2026-01-08 mdebbar@google.com [web] Don't serve files outside of project (flutter/flutter#180699)
2026-01-08 engine-flutter-autoroll@skia.org Roll Skia from 837be28dd218 to e9b3264ade0c (1 revision) (flutter/flutter#180702)
2026-01-08 sokolovskyi.konstantin@gmail.com Add new motion accessibility features to iOS. (flutter/flutter#178102)
2026-01-08 104009581+Saqib198@users.noreply.github.com Fix iOS xattr removal to clear all extended attributes (flutter/flutter#180355)
2026-01-08 bkonyi@google.com [ Widget Preview ] Move widget_preview_scaffold tests to `dev/integration_tests/widget_preview_scaffold` (flutter/flutter#180658)
2026-01-08 30870216+gaaclarke@users.noreply.github.com De-interleaves engine dart test output (flutter/flutter#180651)
2026-01-08 zhongliu88889@gmail.com [web] Fix SemanticsService.announce not working inside dialogs (flutter/flutter#179958)
2026-01-08 engine-flutter-autoroll@skia.org Roll Skia from 42233226ac56 to 837be28dd218 (2 revisions) (flutter/flutter#180693)
2026-01-08 iinozemtsev@google.com Roll Dart SDK to 3.11.0-296.2.beta (flutter/flutter#180685)
2026-01-08 116356835+AbdeMohlbi@users.noreply.github.com Improve code quality in `AndroidTouchProcessorTest.java` (flutter/flutter#180583)
2026-01-08 1961493+harryterkelsen@users.noreply.github.com Revert "Reverts "[reland] Unify canvas creation and Surface code in S…kwasm and CanvasKit (#179473)" (#180152)" (flutter/flutter#180610)
2026-01-08 engine-flutter-autoroll@skia.org Roll Skia from a0c407bce408 to 42233226ac56 (4 revisions) (flutter/flutter#180688)
2026-01-08 bkonyi@google.com [ Tool ] Fix flake in overall_experience_test.dart (flutter/flutter#180655)
2026-01-08 engine-flutter-autoroll@skia.org Roll Packages from 9705815 to 039a026 (6 revisions) (flutter/flutter#180684)
2026-01-08 104147021+MohammedTarigg@users.noreply.github.com flutter_tools: Auto-generate ExportOptions.plist for manual iOS code signing (flutter/flutter#177888)
2026-01-08 engine-flutter-autoroll@skia.org Roll Skia from 58837e160874 to a0c407bce408 (2 revisions) (flutter/flutter#180679)
2026-01-08 engine-flutter-autoroll@skia.org Roll Skia from 1e3266fdba86 to 58837e160874 (1 revision) (flutter/flutter#180677)
2026-01-08 engine-flutter-autoroll@skia.org Roll Skia from 3c47ea10638f to 1e3266fdba86 (4 revisions) (flutter/flutter#180675)
2026-01-08 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from dTvN_JVSCfGFRasvH... to rxeg-6UB678HKJ4UQ... (flutter/flutter#180673)
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants