From 756dcc11180b0de60087a72cf6702ec0dc61f877 Mon Sep 17 00:00:00 2001 From: Valentin Vignal <32538273+ValentinVignal@users.noreply.github.com> Date: Fri, 15 Mar 2024 21:56:45 +0800 Subject: [PATCH] [go_router] Use `leak_tracker_flutter_testing` (#6210) --- packages/go_router/CHANGELOG.md | 5 +++-- packages/go_router/example/pubspec.yaml | 4 ++-- packages/go_router/lib/src/builder.dart | 7 +++++++ packages/go_router/pubspec.yaml | 6 +++--- packages/go_router/test/builder_test.dart | 1 + .../test/custom_transition_page_test.dart | 5 +++++ packages/go_router/test/delegate_test.dart | 18 ++++++++++++++++-- packages/go_router/test/extension_test.dart | 1 + packages/go_router/test/extra_codec_test.dart | 2 ++ packages/go_router/test/go_router_test.dart | 18 ++++++++++++++++++ .../test/helpers/error_screen_helpers.dart | 1 + packages/go_router/test/inherited_test.dart | 1 + packages/go_router/test/name_case_test.dart | 1 + packages/go_router/test/parser_test.dart | 1 + packages/go_router/test/route_data_test.dart | 9 +++++++++ .../go_router/test/routing_config_test.dart | 6 ++++++ packages/go_router/test/test_helpers.dart | 8 ++++++++ 17 files changed, 85 insertions(+), 9 deletions(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 3b01fb1dbb..73e6fe03ae 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,6 +1,7 @@ -## NEXT +## 13.2.1 -* Updates minimum supported SDK version to Flutter 3.13/Dart 3.1. +- Updates minimum supported SDK version to Flutter 3.16/Dart 3.2. +- Fixes memory leaks. ## 13.2.0 diff --git a/packages/go_router/example/pubspec.yaml b/packages/go_router/example/pubspec.yaml index 5eb27e8536..eb4fd432ea 100644 --- a/packages/go_router/example/pubspec.yaml +++ b/packages/go_router/example/pubspec.yaml @@ -4,8 +4,8 @@ version: 3.0.1 publish_to: none environment: - sdk: ^3.1.0 - flutter: ">=3.13.0" + sdk: ^3.2.0 + flutter: ">=3.16.0" dependencies: adaptive_navigation: ^0.0.4 diff --git a/packages/go_router/lib/src/builder.dart b/packages/go_router/lib/src/builder.dart index bcd79c48c1..49b6372c69 100644 --- a/packages/go_router/lib/src/builder.dart +++ b/packages/go_router/lib/src/builder.dart @@ -189,6 +189,13 @@ class _CustomNavigatorState extends State<_CustomNavigator> { _pages = null; } + @override + void dispose() { + _controller?.dispose(); + _registry.dispose(); + super.dispose(); + } + void _updatePages(BuildContext context) { assert(_pages == null); final List> pages = >[]; diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index b93509a97a..3974ba2760 100644 --- a/packages/go_router/pubspec.yaml +++ b/packages/go_router/pubspec.yaml @@ -1,13 +1,13 @@ name: go_router description: A declarative router for Flutter based on Navigation 2 supporting deep linking, data-driven routes and more -version: 13.2.0 +version: 13.2.1 repository: https://github.com/flutter/packages/tree/main/packages/go_router issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22 environment: - sdk: ">=3.1.0 <4.0.0" - flutter: ">=3.13.0" + sdk: ">=3.2.0 <4.0.0" + flutter: ">=3.16.0" dependencies: collection: ^1.15.0 diff --git a/packages/go_router/test/builder_test.dart b/packages/go_router/test/builder_test.dart index f03c090a7b..7b260d5250 100644 --- a/packages/go_router/test/builder_test.dart +++ b/packages/go_router/test/builder_test.dart @@ -371,6 +371,7 @@ void main() { ), ], ); + addTearDown(goRouter.dispose); await tester.pumpWidget(MaterialApp.router( routerConfig: goRouter, diff --git a/packages/go_router/test/custom_transition_page_test.dart b/packages/go_router/test/custom_transition_page_test.dart index c0dac88d0c..710d9d19c6 100644 --- a/packages/go_router/test/custom_transition_page_test.dart +++ b/packages/go_router/test/custom_transition_page_test.dart @@ -22,6 +22,7 @@ void main() { ), ], ); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: router, @@ -35,6 +36,7 @@ void main() { (WidgetTester tester) async { final ValueNotifier showHomeValueNotifier = ValueNotifier(false); + addTearDown(showHomeValueNotifier.dispose); await tester.pumpWidget( MaterialApp( home: ValueListenableBuilder( @@ -83,6 +85,7 @@ void main() { testWidgets('NoTransitionPage does not apply any reverse transition', (WidgetTester tester) async { final ValueNotifier showHomeValueNotifier = ValueNotifier(true); + addTearDown(showHomeValueNotifier.dispose); await tester.pumpWidget( MaterialApp( home: ValueListenableBuilder( @@ -139,6 +142,7 @@ void main() { ), ], ); + addTearDown(router.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: router)); expect(find.byKey(homeKey), findsOneWidget); router.push('/dismissible-modal'); @@ -176,6 +180,7 @@ void main() { ), ], ); + addTearDown(router.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: router)); expect(find.byKey(homeKey), findsOneWidget); diff --git a/packages/go_router/test/delegate_test.dart b/packages/go_router/test/delegate_test.dart index 9b7a99f13f..6513a1c966 100644 --- a/packages/go_router/test/delegate_test.dart +++ b/packages/go_router/test/delegate_test.dart @@ -12,6 +12,7 @@ import 'test_helpers.dart'; Future createGoRouter( WidgetTester tester, { Listenable? refreshListenable, + bool dispose = true, }) async { final GoRouter router = GoRouter( initialLocation: '/', @@ -25,6 +26,9 @@ Future createGoRouter( ], refreshListenable: refreshListenable, ); + if (dispose) { + addTearDown(router.dispose); + } await tester.pumpWidget(MaterialApp.router( routerConfig: router, )); @@ -65,6 +69,7 @@ Future createGoRouterWithStatefulShellRoute( ], builder: mockStackedShellBuilder), ], ); + addTearDown(router.dispose); await tester.pumpWidget(MaterialApp.router( routerConfig: router, )); @@ -287,6 +292,7 @@ void main() { GoRoute(path: '/page-1', builder: (_, __) => const SizedBox()), ], ); + addTearDown(goRouter.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: goRouter, @@ -369,6 +375,7 @@ void main() { builder: (_, __) => const SizedBox()), ], ); + addTearDown(goRouter.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: goRouter, @@ -418,6 +425,7 @@ void main() { GoRoute(path: '/page-1', builder: (_, __) => const SizedBox()), ], ); + addTearDown(goRouter.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: goRouter, @@ -535,6 +543,7 @@ void main() { ), ], ); + addTearDown(router.dispose); await tester.pumpWidget(MaterialApp.router( routerConfig: router, )); @@ -634,8 +643,13 @@ void main() { testWidgets('dispose unsubscribes from refreshListenable', (WidgetTester tester) async { final FakeRefreshListenable refreshListenable = FakeRefreshListenable(); - final GoRouter goRouter = - await createGoRouter(tester, refreshListenable: refreshListenable); + addTearDown(refreshListenable.dispose); + + final GoRouter goRouter = await createGoRouter( + tester, + refreshListenable: refreshListenable, + dispose: false, + ); await tester.pumpWidget(Container()); goRouter.dispose(); expect(refreshListenable.unsubscribed, true); diff --git a/packages/go_router/test/extension_test.dart b/packages/go_router/test/extension_test.dart index 97e5401bad..0f314edbf6 100644 --- a/packages/go_router/test/extension_test.dart +++ b/packages/go_router/test/extension_test.dart @@ -26,6 +26,7 @@ void main() { builder: (_, __) => const SizedBox()) ], ); + addTearDown(router.dispose); await tester.pumpWidget(MaterialApp.router( routerConfig: router, )); diff --git a/packages/go_router/test/extra_codec_test.dart b/packages/go_router/test/extra_codec_test.dart index 3c858cad17..35291db978 100644 --- a/packages/go_router/test/extra_codec_test.dart +++ b/packages/go_router/test/extra_codec_test.dart @@ -32,6 +32,8 @@ void main() { return null; }, ); + + addTearDown(router.dispose); final SimpleDependency dependency = SimpleDependency(); addTearDown(() => dependency.dispose()); diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart index 852e7d9fec..776e9a4d68 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -968,6 +968,8 @@ void main() { testWidgets('does not crash when inherited widget changes', (WidgetTester tester) async { final ValueNotifier notifier = ValueNotifier('initial'); + + addTearDown(notifier.dispose); final List routes = [ GoRoute( path: '/', @@ -985,6 +987,7 @@ void main() { final GoRouter router = GoRouter( routes: routes, ); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: router, @@ -3212,6 +3215,7 @@ void main() { (WidgetTester tester) async { final GoRouterNamedLocationSpy router = GoRouterNamedLocationSpy(routes: routes); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: router, @@ -3230,6 +3234,7 @@ void main() { testWidgets('calls [go] on closest GoRouter', (WidgetTester tester) async { final GoRouterGoSpy router = GoRouterGoSpy(routes: routes); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: router, @@ -3247,6 +3252,7 @@ void main() { testWidgets('calls [goNamed] on closest GoRouter', (WidgetTester tester) async { final GoRouterGoNamedSpy router = GoRouterGoNamedSpy(routes: routes); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: router, @@ -3268,6 +3274,7 @@ void main() { testWidgets('calls [push] on closest GoRouter', (WidgetTester tester) async { final GoRouterPushSpy router = GoRouterPushSpy(routes: routes); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: router, @@ -3285,6 +3292,7 @@ void main() { testWidgets('calls [push] on closest GoRouter and waits for result', (WidgetTester tester) async { final GoRouterPushSpy router = GoRouterPushSpy(routes: routes); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routeInformationProvider: router.routeInformationProvider, @@ -3305,6 +3313,7 @@ void main() { testWidgets('calls [pushNamed] on closest GoRouter', (WidgetTester tester) async { final GoRouterPushNamedSpy router = GoRouterPushNamedSpy(routes: routes); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: router, @@ -3326,6 +3335,7 @@ void main() { testWidgets('calls [pushNamed] on closest GoRouter and waits for result', (WidgetTester tester) async { final GoRouterPushNamedSpy router = GoRouterPushNamedSpy(routes: routes); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routeInformationProvider: router.routeInformationProvider, @@ -3349,6 +3359,7 @@ void main() { testWidgets('calls [pop] on closest GoRouter', (WidgetTester tester) async { final GoRouterPopSpy router = GoRouterPopSpy(routes: routes); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: router, @@ -3363,6 +3374,7 @@ void main() { testWidgets('calls [pop] on closest GoRouter with result', (WidgetTester tester) async { final GoRouterPopSpy router = GoRouterPopSpy(routes: routes); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( routerConfig: router, @@ -4315,6 +4327,7 @@ void main() { GoRoute(path: '/a', builder: (_, __) => const DummyScreen()), ], ); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( @@ -4377,6 +4390,7 @@ void main() { ), ], ); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( @@ -4443,6 +4457,7 @@ void main() { ), ], ); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( @@ -4499,6 +4514,7 @@ void main() { ), ], ); + addTearDown(router.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: router)); @@ -4568,6 +4584,7 @@ void main() { ), ], ); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( @@ -4638,6 +4655,7 @@ void main() { ), ], ); + addTearDown(router.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: router)); diff --git a/packages/go_router/test/helpers/error_screen_helpers.dart b/packages/go_router/test/helpers/error_screen_helpers.dart index 26822e2733..56f03e9fd3 100644 --- a/packages/go_router/test/helpers/error_screen_helpers.dart +++ b/packages/go_router/test/helpers/error_screen_helpers.dart @@ -42,6 +42,7 @@ WidgetTesterCallback testClickingTheButtonRedirectsToRoot({ ), ], ); + addTearDown(router.dispose); await tester.pumpWidget(appRouterBuilder(router)); await tester.tap(buttonFinder); await tester.pumpAndSettle(); diff --git a/packages/go_router/test/inherited_test.dart b/packages/go_router/test/inherited_test.dart index 6c6d965ceb..a5902d47df 100644 --- a/packages/go_router/test/inherited_test.dart +++ b/packages/go_router/test/inherited_test.dart @@ -97,6 +97,7 @@ void main() { ) ], ); + addTearDown(router.dispose); await tester.pumpWidget( MaterialApp.router( diff --git a/packages/go_router/test/name_case_test.dart b/packages/go_router/test/name_case_test.dart index 6e3f067197..d2323c3465 100644 --- a/packages/go_router/test/name_case_test.dart +++ b/packages/go_router/test/name_case_test.dart @@ -25,6 +25,7 @@ void main() { ), ], ); + addTearDown(router.dispose); // run MaterialApp, initial screen path is '/' -> ScreenA await tester.pumpWidget( diff --git a/packages/go_router/test/parser_test.dart b/packages/go_router/test/parser_test.dart index 47ea84c4f9..2d42de859e 100644 --- a/packages/go_router/test/parser_test.dart +++ b/packages/go_router/test/parser_test.dart @@ -27,6 +27,7 @@ void main() { redirectLimit: redirectLimit, redirect: redirect, ); + addTearDown(router.dispose); await tester.pumpWidget(MaterialApp.router( routerConfig: router, )); diff --git a/packages/go_router/test/route_data_test.dart b/packages/go_router/test/route_data_test.dart index dc75fecd18..1308d42635 100644 --- a/packages/go_router/test/route_data_test.dart +++ b/packages/go_router/test/route_data_test.dart @@ -199,6 +199,7 @@ void main() { initialLocation: '/build', routes: _routes, ); + addTearDown(goRouter.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter)); expect(find.byKey(const Key('build')), findsOneWidget); expect(find.byKey(const Key('buildPage')), findsNothing); @@ -212,6 +213,7 @@ void main() { initialLocation: '/build-page', routes: _routes, ); + addTearDown(goRouter.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter)); expect(find.byKey(const Key('build')), findsNothing); expect(find.byKey(const Key('buildPage')), findsOneWidget); @@ -229,6 +231,7 @@ void main() { _shellRouteDataBuilder, ], ); + addTearDown(goRouter.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter)); expect(find.byKey(const Key('builder')), findsOneWidget); expect(find.byKey(const Key('page-builder')), findsNothing); @@ -272,6 +275,7 @@ void main() { ), ], ); + addTearDown(goRouter.dispose); await tester.pumpWidget(MaterialApp.router( routerConfig: goRouter, )); @@ -301,6 +305,7 @@ void main() { _shellRouteDataPageBuilder, ], ); + addTearDown(goRouter.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter)); expect(find.byKey(const Key('builder')), findsNothing); expect(find.byKey(const Key('page-builder')), findsOneWidget); @@ -318,6 +323,7 @@ void main() { _statefulShellRouteDataBuilder, ], ); + addTearDown(goRouter.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter)); expect(find.byKey(const Key('builder')), findsOneWidget); expect(find.byKey(const Key('page-builder')), findsNothing); @@ -333,6 +339,7 @@ void main() { _statefulShellRouteDataPageBuilder, ], ); + addTearDown(goRouter.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter)); expect(find.byKey(const Key('builder')), findsNothing); expect(find.byKey(const Key('page-builder')), findsOneWidget); @@ -367,6 +374,7 @@ void main() { initialLocation: '/redirect', routes: _routes, ); + addTearDown(goRouter.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter)); expect(find.byKey(const Key('build')), findsNothing); expect(find.byKey(const Key('buildPage')), findsOneWidget); @@ -380,6 +388,7 @@ void main() { initialLocation: '/redirect-with-state', routes: _routes, ); + addTearDown(goRouter.dispose); await tester.pumpWidget(MaterialApp.router(routerConfig: goRouter)); expect(find.byKey(const Key('build')), findsNothing); expect(find.byKey(const Key('buildPage')), findsNothing); diff --git a/packages/go_router/test/routing_config_test.dart b/packages/go_router/test/routing_config_test.dart index da36885e00..949c25934c 100644 --- a/packages/go_router/test/routing_config_test.dart +++ b/packages/go_router/test/routing_config_test.dart @@ -18,6 +18,7 @@ void main() { redirect: (_, __) => '/', ), ); + addTearDown(config.dispose); final GoRouter router = await createRouterWithRoutingConfig(config, tester); expect(find.text('home'), findsOneWidget); @@ -35,6 +36,7 @@ void main() { ], ), ); + addTearDown(config.dispose); await createRouterWithRoutingConfig(config, tester); expect(find.text('home'), findsOneWidget); @@ -56,6 +58,7 @@ void main() { ], ), ); + addTearDown(config.dispose); final GoRouter router = await createRouterWithRoutingConfig( config, tester, @@ -87,6 +90,7 @@ void main() { ], ), ); + addTearDown(config.dispose); final GoRouter router = await createRouterWithRoutingConfig( config, tester, @@ -120,11 +124,13 @@ void main() { ], ), ); + addTearDown(config.dispose); final GoRouter router = await createRouterWithRoutingConfig( config, tester, errorBuilder: (_, __) => const Text('error'), ); + expect(find.text('home'), findsOneWidget); // Sanity check. router.goNamed('abc'); diff --git a/packages/go_router/test/test_helpers.dart b/packages/go_router/test/test_helpers.dart index 1126178320..e6f69c5078 100644 --- a/packages/go_router/test/test_helpers.dart +++ b/packages/go_router/test/test_helpers.dart @@ -188,6 +188,7 @@ Future createRouter( requestFocus: requestFocus, overridePlatformDefaultLocation: overridePlatformDefaultLocation, ); + addTearDown(goRouter.dispose); await tester.pumpWidget( MaterialApp.router( restorationScopeId: @@ -221,6 +222,7 @@ Future createRouterWithRoutingConfig( requestFocus: requestFocus, overridePlatformDefaultLocation: overridePlatformDefaultLocation, ); + addTearDown(goRouter.dispose); await tester.pumpWidget( MaterialApp.router( restorationScopeId: @@ -335,6 +337,12 @@ class DummyRestorableStatefulWidgetState } } + @override + void dispose() { + _counter.dispose(); + super.dispose(); + } + @override Widget build(BuildContext context) => Container(); }