Skip to content

[go_router] Activate leak testing #7546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 7, 2024
Merged

[go_router] Activate leak testing #7546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c04b359
test: Activate leak tracking for go router
ValentinVignal Aug 29, 2024
2a2f819
test: Fix leaks in tests
ValentinVignal Aug 29, 2024
869d489
doc: Add license
ValentinVignal Aug 30, 2024
fb4a414
test: Don't ignore leaks created by test helpers
ValentinVignal Aug 30, 2024
246857a
bug: Fix tests on chrome
ValentinVignal Sep 9, 2024
d41a8ea
ci: Allow leak_tracker_flutter_testing unpinned dependency
ValentinVignal Sep 13, 2024
ffe76a6
test: Fix memory leaks in new tests
ValentinVignal Oct 15, 2024
5964d20
Merge branch 'main' into go-router/activate-leak-tracking
chunhtai Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/go_router/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ dev_dependencies:
flutter_test:
sdk: flutter
io: ^1.0.4
leak_tracker_flutter_testing: any
Copy link
Contributor

Choose a reason for hiding this comment

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

can you pin a version instead of any?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, the documentation recommends not to pin any number: see https://github.com/dart-lang/leak_tracker/blob/main/doc%2Fleak_tracking%2FDETECT.md

Do you still want me to pin one after reading this?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh i wasn't aware that is the guideline, thanks for pointing it out. LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like the pipeline Linux repo_checks is not happy with this. Should it be ignored (and how?) ?
Or is there something else we can do about it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should ignore it given that is how leak tracking suggests.

I think you need to modify packages/script/configs/allowed_unpinned_deps.yaml

cc @stuartmorgan

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to modify packages/script/configs/allowed_unpinned_deps.yaml

It looks like the tooling doesn't check for any specifically, which it should, so that should work. (At some point I'll need to fix that and split this exception out into its own file, since in general we don't want to allow any.)

path: ^1.8.2

topics:
Expand Down
1 change: 1 addition & 0 deletions packages/go_router/test/extra_codec_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ void main() {
expect(find.text(initialString), findsOneWidget);

await tester.restartAndRestore();
addTearDown(tester.binding.restorationManager.dispose);

await tester.pumpAndSettle();
expect(find.text(initialString), findsOneWidget);
Expand Down
13 changes: 13 additions & 0 deletions packages/go_router/test/flutter_test_config.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';

import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';

Future<void> testExecutable(FutureOr<void> Function() testMain) async {
LeakTesting.enable();
LeakTracking.warnForUnsupportedPlatforms = false;
await testMain();
}
1 change: 1 addition & 0 deletions packages/go_router/test/go_router_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5458,6 +5458,7 @@ void main() {
),
],
);
addTearDown(router.dispose);
await tester.pumpWidget(
MaterialApp.router(
key: UniqueKey(),
Expand Down
6 changes: 6 additions & 0 deletions packages/go_router/test/information_provider_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ void main() {
late final GoRouteInformationProvider provider =
GoRouteInformationProvider(
initialLocation: initialRoute, initialExtra: null);
addTearDown(provider.dispose);
provider.addListener(expectAsync0(() {}));
provider.go(newRoute);
});
Expand All @@ -26,6 +27,7 @@ void main() {
late final GoRouteInformationProvider provider =
GoRouteInformationProvider(
initialLocation: initialRoute, initialExtra: null);
addTearDown(provider.dispose);
provider.addListener(expectAsync0(() {}));
provider
.didPushRouteInformation(RouteInformation(uri: Uri.parse(newRoute)));
Expand All @@ -41,6 +43,7 @@ void main() {
late final GoRouteInformationProvider provider =
GoRouteInformationProvider(
initialLocation: initialRoute, initialExtra: null);
addTearDown(provider.dispose);
provider.addListener(expectAsync0(() {}));
provider.didPushRouteInformation(
RouteInformation(uri: Uri.parse(expectedUriString)));
Expand All @@ -60,6 +63,7 @@ void main() {
late final GoRouteInformationProvider provider =
GoRouteInformationProvider(
initialLocation: initialRoute, initialExtra: null);
addTearDown(provider.dispose);
provider.addListener(expectAsync0(() {}));
provider.didPushRouteInformation(
RouteInformation(uri: Uri.parse(expectedUriString)));
Expand All @@ -78,6 +82,7 @@ void main() {
initialLocation: initialRoute,
initialExtra: null,
routerNeglect: true);
addTearDown(provider.dispose);
provider.addListener(expectAsync0(() {}));
provider.go(newRoute);
provider.routerReportsNewRouteInformation(
Expand All @@ -94,6 +99,7 @@ void main() {
late final GoRouteInformationProvider provider =
GoRouteInformationProvider(
initialLocation: initialRoute, initialExtra: null);
addTearDown(provider.dispose);
provider.addListener(expectAsync0(() {}));
provider.go(newRoute);
provider.routerReportsNewRouteInformation(
Expand Down
1 change: 1 addition & 0 deletions script/configs/allowed_unpinned_deps.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
- io
- js
- json_serializable
- leak_tracker_flutter_testing
- lints
- logging
- markdown
Expand Down