packages
packages copied to clipboard
[go_router] Activate leak testing
Pre-launch Checklist
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene page, which explains my responsibilities.
- [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use
dart format.) - [x] I signed the CLA.
- [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g.
[shared_preferences] - [ ] I linked to at least one issue that this PR fixes in the description above.
- [ ] I updated
pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [ ] I updated
CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes. - [x] I updated/added relevant documentation (doc comments with
///). - [x] I added new tests to check the change I am making, or this PR is test-exempt.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
cc @polina-c
I didn't modify the version number and the CHANGELOG, should I do it or is it not necessary for this?
Thanks for pushing it! Looks good to me. Will let @chunhtai to stamp.
override reason: test only.
I am not sure what happen to the ci, can you try to rebase and see if that passes the ci.
looks like the test was trying to run
flutter test --color --platform=chrome --web-renderer=canvaskit" in /b/s/w/ir/x/w/packages/packages/go_router
It looks like the signature needs to be
Future<void> testExecutable(FutureOr<void> Function() testMain)
@polina-c do you want me to make a PR to update https://github.com/dart-lang/leak_tracker/blob/main/doc%2Fleak_tracking%2FDETECT.md ?
It looks like the signature needs to be
Future<void> testExecutable(FutureOr<void> Function() testMain)@polina-c do you want me to make a PR to update https://github.com/dart-lang/leak_tracker/blob/main/doc%2Fleak_tracking%2FDETECT.md ?
Hm, interesting. Yes, please. Thank you!
I updated the documentation in https://github.com/dart-lang/leak_tracker/pull/245
looks like here is a ci failure https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8737284438128899377/+/u/Run_package_tests/pubspec_validation/stdout
Hm. Leak tracker is not recognized as local dependency, while it is part of Flutter.
Leak tracker is not recognized as local dependency, while it is part of Flutter.
It isn't part of the Flutter SDK, otherwise the dependency would be an SDK dependency and the repo tooling wouldn't flag it. Local means "in this repository", which it also isn't. It's just a random package from pub as far as the repo tooling is concerned. (I'd be happy to take a PR that makes the check cross-reference by querying the pub.dev API for any package that's not SDK and not local, and allowing anything with a Dart or Flutter official publisher.)
Thus https://github.com/flutter/packages/pull/7546/files#r1757634961
Thank you for the insights.
https://github.com/flutter/packages/commit/90ee2ca1743ad333e64a261e71620fba2ebcdac5 made the ci pass
@chunhtai I see you added the label "waiting for tree to go green"? Is there a bot that will automatically merge this PR once it is ready (like the flutter repository)? Or can I merge it by hand if all the checks are successful?
@chunhtai I see you added the label "waiting for tree to go green"? Is there a bot that will automatically merge this PR once it is ready (like the flutter repository)? Or can I merge it by hand if all the checks are successful?
I am not sure why it doesn't trigger auto merge, let me update the branch and see if it picks up this time
@chunhtai packages/packages/pointer_interceptor/ tests keep timing out. Is it from my changes? It looks unrelated to me
When you run the tests, is tree red or green: https://flutter-dashboard.appspot.com/#/build If red, wait for it to be green and then rerun. If green, it may be leak tracker effect. Leak tracker may need to be turned off for these tests.
I'm super confused. The failing package is packages/video_player/video_player_android, packages/video_player/video_player_android/example/integration_test/video_player_test.dart.
But is passes on my local. Does anyone have any insight about that?
I'm super confused. The failing package is
packages/video_player/video_player_android, But is passes on my local. Does anyone have any insight about that?
Thank you for persistence!
May be bots fail on different OS than than your local?
When I face such things, I temporarily add debug logs to the failing test, so that I can see what is going on on bots, when they fail. You also can temporarily adjust ci.yaml in the root to reduce number of bots to run, and remove non-needed tests to get the results faster. And you may want to create separate draft PR for the experiments, to avoid noise on this.
Does this pr touches video_player package? if not, this may just be an outdated branch, can you try rebase?
Does this pr touches video_player package? if not, this may just be an outdated branch, can you try rebase?
No it doesn't, that's why I'm having hard time understanding the issue 😅
It looks like the failing test is also failing on main. After it is fixed, this PR should be good to go