Skip to content

Conversation

@polina-c
Copy link
Contributor

No description provided.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

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

@github-actions github-actions bot added c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels. d: docs/ flutter/flutter/docs, for contributors labels Oct 23, 2024
@polina-c polina-c requested a review from Piinks October 23, 2024 01:42
@polina-c
Copy link
Contributor Author

There is no tests, because this PR updates testing code and documentation only.

and there is no simple way to either test what is needed
in a recommended way or to dispose the objects.

2. Thrown exceptions
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a common test case, is there guidance on how to handle these without opting out of leak tracking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not yet

Comment on lines 31 to 32
the project to make sure disposables are disposed
in exceptional code paths was not prioritized yet.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an issue tracking this that folks can refer to for updates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created: #157470

- [Windows framework_tests_libraries_leak_tracking](https://github.com/flutter/flutter/blob/9441f8f6c806fb0a3b7d058a40b5e59c373e6055/.ci.yaml#L5553)
- [Windows framework_tests_widgets_leak_tracking](https://github.com/flutter/flutter/blob/9441f8f6c806fb0a3b7d058a40b5e59c373e6055/.ci.yaml#L5640C11-L5640C56)

To enable leak tracking for flutter tests, pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this for local testing?

Copy link
Contributor Author

@polina-c polina-c Oct 23, 2024

Choose a reason for hiding this comment

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

for both local testing and bots

clarified

Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

Also would be good to include an example of the failure output from a leak tracking test so folks can identify it.

@polina-c polina-c requested a review from Piinks October 23, 2024 21:29
@stuartmorgan-g
Copy link
Contributor

There is no tests, because this PR updates testing code and documentation only.

Isn't this changing user-facing behavior of a user-facing package that is intended for use in tests, rather than code that is just part of one of our own tests? If it's the former, that's not considered a test for exemption purposes.

@polina-c
Copy link
Contributor Author

There is no tests, because this PR updates testing code and documentation only.

Isn't this changing user-facing behavior of a user-facing package that is intended for use in tests, rather than code that is just part of one of our own tests? If it's the former, that's not considered a test for exemption purposes.

The modified code is under 'test'.
It changes information that developers of Flutter Framework will see when they run unit tests of Flutter Framework.
All other people will not get any change.
All other code of flutter_test_config.dart is not test covered.

Does it help?

@stuartmorgan-g
Copy link
Contributor

test-exempt: is a test

(Sorry about that, my brain got confused by the flutter_test in the filename, and read the path as packages/flutter_test/... rather than packages/flutter/test/.... Which I should have realized was wrong since it didn't have a /lib/.)

Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

LGTM

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 26, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Oct 26, 2024
Roll Flutter from 4faa4a4 to 5a11904 (67 revisions)

flutter/flutter@4faa4a4...5a11904

2024-10-26 30870216+gaaclarke@users.noreply.github.com Relands "Wide gamut framework gradient test (#153976)" (flutter/flutter#157643)
2024-10-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7c5c5fe5c84d to c9b8ac96f6ce (3 revisions) (flutter/flutter#157662)
2024-10-26 32538273+ValentinVignal@users.noreply.github.com Add test for `navigator_state.restorable_push_and_remove_until.0.dart` (flutter/flutter#157595)
2024-10-26 matanlurey@users.noreply.github.com Tighten up `throwToolExit`, explain when to use it. (flutter/flutter#157561)
2024-10-26 matanlurey@users.noreply.github.com Remove extraneous `throw`. (flutter/flutter#157658)
2024-10-26 32538273+ValentinVignal@users.noreply.github.com Add tests for `navigator.restorable_push.0.dart` (flutter/flutter#157492)
2024-10-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 43e4d9a30666 to 7c5c5fe5c84d (1 revision) (flutter/flutter#157644)
2024-10-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5061358e255f to 43e4d9a30666 (1 revision) (flutter/flutter#157637)
2024-10-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from eb867e055790 to 5061358e255f (2 revisions) (flutter/flutter#157623)
2024-10-25 polinach@google.com Create flutter specific leak troubleshooting guidance. (flutter/flutter#157396)
2024-10-25 katelovett@google.com Update CupertinoNavigationBar to support large layout (flutter/flutter#157133)
2024-10-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 38e9be1f74fa to eb867e055790 (3 revisions) (flutter/flutter#157613)
2024-10-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Wide gamut framework gradient test (#153976)" (flutter/flutter#157615)
2024-10-25 30870216+gaaclarke@users.noreply.github.com Wide gamut framework gradient test (flutter/flutter#153976)
2024-10-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from b413d9996c86 to 38e9be1f74fa (2 revisions) (flutter/flutter#157604)
2024-10-25 engine-flutter-autoroll@skia.org Roll Packages from a556f0f to e0c4f55 (2 revisions) (flutter/flutter#157605)
2024-10-25 jonahwilliams@google.com Support backdrop key in flutter framework. (flutter/flutter#157278)
2024-10-25 reidbaker@google.com Add 3.24.4 changelog to master (flutter/flutter#157600)
2024-10-25 mohellebiabdessalem@gmail.com Update flutter.groovy to catch unknown task exception when finding api task (flutter/flutter#157282)
2024-10-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from c4b0184c8783 to b413d9996c86 (1 revision) (flutter/flutter#157580)
2024-10-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from b1c2ba8c4d52 to c4b0184c8783 (1 revision) (flutter/flutter#157578)
2024-10-25 32538273+ValentinVignal@users.noreply.github.com Add test for `build_owner.0.dart` (flutter/flutter#157499)
2024-10-25 32538273+ValentinVignal@users.noreply.github.com Add tests  for `focusable_action_detector.0.dart` (flutter/flutter#157575)
2024-10-25 32538273+ValentinVignal@users.noreply.github.com Add test for `navigator.restorable_push_and_remove_until.0.dart` (flutter/flutter#157487)
2024-10-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 29440ed1e225 to b1c2ba8c4d52 (1 revision) (flutter/flutter#157572)
2024-10-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from 88716d804aef to 29440ed1e225 (1 revision) (flutter/flutter#157569)
2024-10-25 engine-flutter-autoroll@skia.org Roll Flutter Engine from b8b28c80a737 to 88716d804aef (2 revisions) (flutter/flutter#157567)
2024-10-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from 48ff670d256b to b8b28c80a737 (2 revisions) (flutter/flutter#157564)
2024-10-24 matanlurey@users.noreply.github.com Use discenrable characters (replace `' � � '` in error logs) (flutter/flutter#157548)
2024-10-24 matanlurey@users.noreply.github.com Remove unused `PubDependenciesProjectValidator`. (flutter/flutter#157516)
2024-10-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Upgrade tests to AGP 8.7/Gradle 8.10.2/Kotlin 1.8.10 (#157032)" (flutter/flutter#157559)
2024-10-24 jonahwilliams@google.com Mark mac impeller as bringup. (flutter/flutter#157551)
2024-10-24 tessertaha@gmail.com Deprecate `ThemeData.dialogBackgroundColor` in favor of `DialogThemeData.backgroundColor` (flutter/flutter#155072)
2024-10-24 34871572+gmackall@users.noreply.github.com Upgrade tests to AGP 8.7/Gradle 8.10.2/Kotlin 1.8.10 (flutter/flutter#157032)
2024-10-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from 246344f26edc to 48ff670d256b (2 revisions) (flutter/flutter#157544)
2024-10-24 matanlurey@users.noreply.github.com Allow opting out of `.flutter-plugins`, opt-out in `refreshPluginsList`. (flutter/flutter#157527)
2024-10-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reverts "Added a warning if `flutter.groovy` uses a `.flutter-plugins` file. (#157388)" (#157541)" (flutter/flutter#157549)
2024-10-24 50643541+Mairramer@users.noreply.github.com Changes the offset computation to first item for RenderSliverMainAxisGroup (flutter/flutter#154688)
2024-10-24 engine-flutter-autoroll@skia.org Roll Packages from 5e03bb1 to a556f0f (7 revisions) (flutter/flutter#157539)
2024-10-24 737941+loic-sharma@users.noreply.github.com Add partial test for flutter build ios-framework on non-module (flutter/flutter#157482)
2024-10-24 737941+loic-sharma@users.noreply.github.com Add example to SafeArea docs (flutter/flutter#157228)
2024-10-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Added a warning if `flutter.groovy` uses a `.flutter-plugins` file. (#157388)" (flutter/flutter#157541)
2024-10-24 matanlurey@users.noreply.github.com Added a warning if `flutter.groovy` uses a `.flutter-plugins` file. (flutter/flutter#157388)
2024-10-24 engine-flutter-autoroll@skia.org Roll Flutter Engine from be56084344d1 to 246344f26edc (2 revisions) (flutter/flutter#157504)
2024-10-24 104349824+huycozy@users.noreply.github.com Add ability to disable CupertinoSegmentedControl (flutter/flutter#152813)
2024-10-24 tessertaha@gmail.com Update `Tab.height` parameter doc for tab height lower than default (flutter/flutter#157443)
...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. d: docs/ flutter/flutter/docs, for contributors framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants