Enable misc leak tracking#176992
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a new CI target, Windows framework_tests_misc_leak_tracking, to enable leak tracking for the misc subshard. It also adds a flutter_test_config.dart file, presumably to configure leak tracking for tests related to WidgetTester. My review focuses on the correctness of the CI configuration and the new test configuration file. I've identified a critical issue with the location of the new config file which will prevent it from being used, and a couple of medium-severity issues related to maintainability in both the CI config and the Dart code.
| @@ -0,0 +1,44 @@ | |||
| // Copyright 2014 The Flutter Authors. All rights reserved. | |||
There was a problem hiding this comment.
This flutter_test_config.dart file appears to be in the wrong location. For the test runner to pick it up, it must be placed inside a directory that contains tests. For the flutter_test package, this would be packages/flutter_test/test/. In its current location at packages/flutter_test/, it will not be discovered and will have no effect.
| bool _isLeakTrackingEnabled() { | ||
| if (kIsWeb) { | ||
| return false; | ||
| } | ||
| // The values can be different, one is compile time, another is run time. | ||
| return const bool.fromEnvironment('LEAK_TRACKING') || | ||
| (bool.tryParse(Platform.environment['LEAK_TRACKING'] ?? '') ?? false); | ||
| } |
There was a problem hiding this comment.
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Hey @dkwingsmt is this change still on your radar? |
|
I still want to finish this and am hoping to discuss with @/jtmcdole , who has been busy towards the end of the year. I'll keep following up. |
|
|
||
| - name: Windows framework_tests_misc_leak_tracking | ||
| recipe: flutter/flutter_drone | ||
| timeout: 120 |
There was a problem hiding this comment.
Is this really going to take 2 hours? We have 55 windows bots for 55 tests and a majority of those are 60 minutes.
There was a problem hiding this comment.
I've changed it to 60 to align with framework_tests_misc. Thanks for pointing out.
| } | ||
| // The values can be different, one is compile time, another is run time. | ||
| return const bool.fromEnvironment('LEAK_TRACKING') || | ||
| (bool.tryParse(Platform.environment['LEAK_TRACKING'] ?? '') ?? false); |
There was a problem hiding this comment.
Took me a minute to find that this is the only variable that CI cares about. I assume fromEnvironment is for personal use to set this flag.
Roll Flutter from 13b2b91 to 01d37bc (74 revisions) flutter/flutter@13b2b91...01d37bc 2026-01-07 [email protected] Replace Hybrid Composition wiki page with dev-facing content (flutter/flutter#180642) 2026-01-07 [email protected] Improve code quality in `FlutterActivityTest.java` (flutter/flutter#180585) 2026-01-07 [email protected] Roll Dart SDK to 3.11.0-296.1.beta (flutter/flutter#180633) 2026-01-07 [email protected] Bump target Windows version to 10 (flutter/flutter#180624) 2026-01-07 [email protected] Run hook_user_defines and link_hook integration tests on CI (flutter/flutter#180622) 2026-01-07 [email protected] Fix division by zero in RenderTable intrinsic size methods (flutter/flutter#178217) 2026-01-07 [email protected] Remove more `requires 24` anotations (flutter/flutter#180116) 2026-01-07 [email protected] Roll Skia from 3971dbb85d45 to c5359a4d720e (1 revision) (flutter/flutter#180631) 2026-01-07 [email protected] Fix TabBar.image does not render at initialIndex for the first time (flutter/flutter#179616) 2026-01-07 [email protected] Roll Skia from 1e7ad625f6f7 to 3971dbb85d45 (3 revisions) (flutter/flutter#180627) 2026-01-07 [email protected] Unpin DDS (flutter/flutter#180571) 2026-01-07 [email protected] Fix DropdownMenuEntry.style not resolved when entry is highlighted (flutter/flutter#178873) 2026-01-07 [email protected] Remove unnecessary `@RequiresApi24` annotations from FlutterFragment methods (flutter/flutter#180117) 2026-01-07 [email protected] Roll Skia from 7fc63228056b to 1e7ad625f6f7 (1 revision) (flutter/flutter#180616) 2026-01-07 [email protected] Roll Fuchsia Linux SDK from QfR2ZFZ5kGTD3raO3... to dTvN_JVSCfGFRasvH... (flutter/flutter#180612) 2026-01-07 [email protected] [ Widget Preview ] Add support for `dart:ffi` imports (flutter/flutter#180586) 2026-01-07 [email protected] Roll Skia from eec90000a899 to 7fc63228056b (2 revisions) (flutter/flutter#180608) 2026-01-07 [email protected] Add --web-define flag for template variable injection in Flutter web builds (flutter/flutter#175805) 2026-01-07 [email protected] docs(engine): update rbe notes (flutter/flutter#180599) 2026-01-06 [email protected] Roll Skia from b6249496c230 to eec90000a899 (3 revisions) (flutter/flutter#180602) 2026-01-06 [email protected] Forward proxy 404 responses to client (flutter/flutter#179858) 2026-01-06 [email protected] Roll Dart SDK from 40a8c6188f7f to d2e3ce177270 (1 revision) (flutter/flutter#180598) 2026-01-06 [email protected] Restore CLI precedence for web headers and HTTPS over web_dev_config.yaml (flutter/flutter#179639) 2026-01-06 [email protected] Roll Skia from f5d9da13d56d to b6249496c230 (1 revision) (flutter/flutter#180596) 2026-01-06 [email protected] Enable misc leak tracking (flutter/flutter#176992) 2026-01-06 [email protected] [hooks] Don't require NDK for Android targets (flutter/flutter#180594) 2026-01-06 [email protected] [skia] Disable legacy non-const SkData APIs (flutter/flutter#179684) 2026-01-06 [email protected] Fix/ios share context menu (flutter/flutter#176199) 2026-01-06 [email protected] Roll Skia from b970aeffa66f to f5d9da13d56d (5 revisions) (flutter/flutter#180588) 2026-01-06 [email protected] Manually bump dependencies (flutter/flutter#180509) 2026-01-06 [email protected] Roll Dart SDK from 8150be8a0e48 to 40a8c6188f7f (2 revisions) (flutter/flutter#180582) 2026-01-06 [email protected] Roll Packages from 12eb764 to d3f860d (2 revisions) (flutter/flutter#180581) 2026-01-06 [email protected] Roll Fuchsia Test Scripts from MHF-UAfO6sVKqSEYk... to nR2ESa1Gd8yPcWo06... (flutter/flutter#180578) 2026-01-06 [email protected] Add tooltip support to PlatformMenuItem and PlatformMenu. (flutter/flutter#180069) 2026-01-06 [email protected] Add DropdownMenuFormField.errorBuilder (flutter/flutter#179345) 2026-01-06 [email protected] Roll Skia from 1845397e11ba to b970aeffa66f (2 revisions) (flutter/flutter#180566) 2026-01-06 [email protected] Don't embed unreferenced assets (flutter/flutter#179251) 2026-01-06 [email protected] Improve documentation about ValueNotifier's behavior (flutter/flutter#179870) 2026-01-06 [email protected] Roll Skia from 904ba00331ca to 1845397e11ba (5 revisions) (flutter/flutter#180558) 2026-01-06 [email protected] Roll Dart SDK from 2fb9ad834c4d to 8150be8a0e48 (1 revision) (flutter/flutter#180557) 2026-01-06 [email protected] Roll Skia from 98c042dde68c to 904ba00331ca (3 revisions) (flutter/flutter#180550) 2026-01-06 [email protected] Roll Dart SDK from ba9f7f790966 to 2fb9ad834c4d (2 revisions) (flutter/flutter#180548) 2026-01-06 [email protected] Roll Fuchsia Linux SDK from ubBGcRaAKWKihQ4ac... to QfR2ZFZ5kGTD3raO3... (flutter/flutter#180547) 2026-01-06 [email protected] Make sure that a DraggableScrollableSheet doesn't crash in 0x0 enviro… (flutter/flutter#180433) 2026-01-06 [email protected] Make sure that a ColorFiltered doesn't crash 0x0 environment (flutter/flutter#180307) 2026-01-06 [email protected] Make sure that a FadeInImage doesn't crash in 0x0 environment (flutter/flutter#180495) ...
…art` (#180600) 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 - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. 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](https://developers.google.com/gemini-code-assist/docs/review-github-code). 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Following #176992, this PR fixes all leaks in `flutter_test` tests. A new method is added to reliably dispose the root layer of render views. Feel free to let me know if you have suggestions for the name! ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. 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](https://developers.google.com/gemini-code-assist/docs/review-github-code). 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Roll Flutter from 13b2b91 to 01d37bc (74 revisions) flutter/flutter@13b2b91...01d37bc 2026-01-07 [email protected] Replace Hybrid Composition wiki page with dev-facing content (flutter/flutter#180642) 2026-01-07 [email protected] Improve code quality in `FlutterActivityTest.java` (flutter/flutter#180585) 2026-01-07 [email protected] Roll Dart SDK to 3.11.0-296.1.beta (flutter/flutter#180633) 2026-01-07 [email protected] Bump target Windows version to 10 (flutter/flutter#180624) 2026-01-07 [email protected] Run hook_user_defines and link_hook integration tests on CI (flutter/flutter#180622) 2026-01-07 [email protected] Fix division by zero in RenderTable intrinsic size methods (flutter/flutter#178217) 2026-01-07 [email protected] Remove more `requires 24` anotations (flutter/flutter#180116) 2026-01-07 [email protected] Roll Skia from 3971dbb85d45 to c5359a4d720e (1 revision) (flutter/flutter#180631) 2026-01-07 [email protected] Fix TabBar.image does not render at initialIndex for the first time (flutter/flutter#179616) 2026-01-07 [email protected] Roll Skia from 1e7ad625f6f7 to 3971dbb85d45 (3 revisions) (flutter/flutter#180627) 2026-01-07 [email protected] Unpin DDS (flutter/flutter#180571) 2026-01-07 [email protected] Fix DropdownMenuEntry.style not resolved when entry is highlighted (flutter/flutter#178873) 2026-01-07 [email protected] Remove unnecessary `@RequiresApi24` annotations from FlutterFragment methods (flutter/flutter#180117) 2026-01-07 [email protected] Roll Skia from 7fc63228056b to 1e7ad625f6f7 (1 revision) (flutter/flutter#180616) 2026-01-07 [email protected] Roll Fuchsia Linux SDK from QfR2ZFZ5kGTD3raO3... to dTvN_JVSCfGFRasvH... (flutter/flutter#180612) 2026-01-07 [email protected] [ Widget Preview ] Add support for `dart:ffi` imports (flutter/flutter#180586) 2026-01-07 [email protected] Roll Skia from eec90000a899 to 7fc63228056b (2 revisions) (flutter/flutter#180608) 2026-01-07 [email protected] Add --web-define flag for template variable injection in Flutter web builds (flutter/flutter#175805) 2026-01-07 [email protected] docs(engine): update rbe notes (flutter/flutter#180599) 2026-01-06 [email protected] Roll Skia from b6249496c230 to eec90000a899 (3 revisions) (flutter/flutter#180602) 2026-01-06 [email protected] Forward proxy 404 responses to client (flutter/flutter#179858) 2026-01-06 [email protected] Roll Dart SDK from 40a8c6188f7f to d2e3ce177270 (1 revision) (flutter/flutter#180598) 2026-01-06 [email protected] Restore CLI precedence for web headers and HTTPS over web_dev_config.yaml (flutter/flutter#179639) 2026-01-06 [email protected] Roll Skia from f5d9da13d56d to b6249496c230 (1 revision) (flutter/flutter#180596) 2026-01-06 [email protected] Enable misc leak tracking (flutter/flutter#176992) 2026-01-06 [email protected] [hooks] Don't require NDK for Android targets (flutter/flutter#180594) 2026-01-06 [email protected] [skia] Disable legacy non-const SkData APIs (flutter/flutter#179684) 2026-01-06 [email protected] Fix/ios share context menu (flutter/flutter#176199) 2026-01-06 [email protected] Roll Skia from b970aeffa66f to f5d9da13d56d (5 revisions) (flutter/flutter#180588) 2026-01-06 [email protected] Manually bump dependencies (flutter/flutter#180509) 2026-01-06 [email protected] Roll Dart SDK from 8150be8a0e48 to 40a8c6188f7f (2 revisions) (flutter/flutter#180582) 2026-01-06 [email protected] Roll Packages from 12eb764 to d3f860d (2 revisions) (flutter/flutter#180581) 2026-01-06 [email protected] Roll Fuchsia Test Scripts from MHF-UAfO6sVKqSEYk... to nR2ESa1Gd8yPcWo06... (flutter/flutter#180578) 2026-01-06 [email protected] Add tooltip support to PlatformMenuItem and PlatformMenu. (flutter/flutter#180069) 2026-01-06 [email protected] Add DropdownMenuFormField.errorBuilder (flutter/flutter#179345) 2026-01-06 [email protected] Roll Skia from 1845397e11ba to b970aeffa66f (2 revisions) (flutter/flutter#180566) 2026-01-06 [email protected] Don't embed unreferenced assets (flutter/flutter#179251) 2026-01-06 [email protected] Improve documentation about ValueNotifier's behavior (flutter/flutter#179870) 2026-01-06 [email protected] Roll Skia from 904ba00331ca to 1845397e11ba (5 revisions) (flutter/flutter#180558) 2026-01-06 [email protected] Roll Dart SDK from 2fb9ad834c4d to 8150be8a0e48 (1 revision) (flutter/flutter#180557) 2026-01-06 [email protected] Roll Skia from 98c042dde68c to 904ba00331ca (3 revisions) (flutter/flutter#180550) 2026-01-06 [email protected] Roll Dart SDK from ba9f7f790966 to 2fb9ad834c4d (2 revisions) (flutter/flutter#180548) 2026-01-06 [email protected] Roll Fuchsia Linux SDK from ubBGcRaAKWKihQ4ac... to QfR2ZFZ5kGTD3raO3... (flutter/flutter#180547) 2026-01-06 [email protected] Make sure that a DraggableScrollableSheet doesn't crash in 0x0 enviro… (flutter/flutter#180433) 2026-01-06 [email protected] Make sure that a ColorFiltered doesn't crash 0x0 environment (flutter/flutter#180307) 2026-01-06 [email protected] Make sure that a FadeInImage doesn't crash in 0x0 environment (flutter/flutter#180495) ...
Following flutter#176992, this PR fixes all leaks in `flutter_test` tests. A new method is added to reliably dispose the root layer of render views. Feel free to let me know if you have suggestions for the name! ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. 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](https://developers.google.com/gemini-code-assist/docs/review-github-code). 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This PR adds a new subshard that runs leak tracking tests on flutter/misc.
This is used to address a problem faced by PR #176519, which fixes a leak in
WidgetTesterbut is unable to write a unit test.I'm not certain this is the best approach so I'm open to other suggestions.
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-assistbot 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.