From 0cef0aaf358fde6ca9efe5669d0a73774799b2b6 Mon Sep 17 00:00:00 2001 From: perlatus Date: Wed, 7 Jun 2017 21:32:29 -0400 Subject: [PATCH] Check for initialRoute before Navigator.defaultRouteName (#10216) * Check for initialRoute before Navigator.defaultRouteName * Default initialRoute to Navigator.defaultRouteName * Take suggestions from code review * Add test for old and new routes behavior * Revert "Add test for old and new routes behavior" This reverts commit 282fb64b165ed532583e9a5d2e4debe29469fba4. * Retry: without dartfmt, with dartanalyzer * Rename tests, check the routes are taken * Fix flutter analyze --flutter-repo warnings * Add test for initial vs default route * Update test and fix analyzer warnings * Add test for initial route only being used initially --- packages/flutter/lib/src/material/app.dart | 11 +-- packages/flutter/test/material/app_test.dart | 72 ++++++++++++++++++++ 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/packages/flutter/lib/src/material/app.dart b/packages/flutter/lib/src/material/app.dart index 425944da85..a860860d86 100644 --- a/packages/flutter/lib/src/material/app.dart +++ b/packages/flutter/lib/src/material/app.dart @@ -42,8 +42,9 @@ class MaterialApp extends StatefulWidget { /// Creates a MaterialApp. /// /// At least one of [home], [routes], or [onGenerateRoute] must be - /// given. If only [routes] is given, it must include an entry for - /// the [Navigator.defaultRouteName] (`'/'`). + /// given. If only [routes] is given, it must include an entry for the + /// [initialRoute], which defaults to [Navigator.defaultRouteName] + /// (`'/'`). /// /// This class creates an instance of [WidgetsApp]. MaterialApp({ @@ -53,7 +54,7 @@ class MaterialApp extends StatefulWidget { this.theme, this.home, this.routes: const {}, - this.initialRoute, + this.initialRoute: Navigator.defaultRouteName, this.onGenerateRoute, this.onLocaleChanged, this.navigatorObservers: const [], @@ -65,8 +66,8 @@ class MaterialApp extends StatefulWidget { this.debugShowCheckedModeBanner: true }) : assert(debugShowMaterialGrid != null), assert(routes != null), - assert(!routes.containsKey(Navigator.defaultRouteName) || (home == null)), - assert(routes.containsKey(Navigator.defaultRouteName) || (home != null) || (onGenerateRoute != null)), + assert(!routes.containsKey(initialRoute) || (home == null)), + assert(routes.containsKey(initialRoute) || (home != null) || (onGenerateRoute != null)), super(key: key); /// A one-line description of this app for use in the window manager. diff --git a/packages/flutter/test/material/app_test.dart b/packages/flutter/test/material/app_test.dart index e9b2662214..92249a5ae1 100644 --- a/packages/flutter/test/material/app_test.dart +++ b/packages/flutter/test/material/app_test.dart @@ -143,4 +143,76 @@ void main() { expect(find.text('Home'), findsOneWidget); }); + + testWidgets('Default initialRoute', (WidgetTester tester) async { + await tester.pumpWidget(new MaterialApp(routes: { + '/': (BuildContext context) => const Text('route "/"'), + })); + + expect(find.text('route "/"'), findsOneWidget); + }); + + testWidgets('Custom initialRoute only', (WidgetTester tester) async { + await tester.pumpWidget( + new MaterialApp( + initialRoute: '/a', + routes: { + '/a': (BuildContext context) => const Text('route "/a"'), + }, + ) + ); + + expect(find.text('route "/a"'), findsOneWidget); + }); + + testWidgets('Custom initialRoute along with Navigator.defaultRouteName', (WidgetTester tester) async { + final Map routes = { + '/': (BuildContext context) => const Text('route "/"'), + '/a': (BuildContext context) => const Text('route "/a"'), + '/b': (BuildContext context) => const Text('route "/b"'), + }; + + await tester.pumpWidget( + new MaterialApp( + initialRoute: '/a', + routes: routes, + ) + ); + expect(find.text('route "/"'), findsNothing); + expect(find.text('route "/a"'), findsOneWidget); + expect(find.text('route "/b"'), findsNothing); + }); + + testWidgets('Make sure initialRoute is only used the first time', (WidgetTester tester) async { + final Map routes = { + '/': (BuildContext context) => const Text('route "/"'), + '/a': (BuildContext context) => const Text('route "/a"'), + '/b': (BuildContext context) => const Text('route "/b"'), + }; + + await tester.pumpWidget( + new MaterialApp( + initialRoute: '/a', + routes: routes, + ) + ); + expect(find.text('route "/"'), findsNothing); + expect(find.text('route "/a"'), findsOneWidget); + expect(find.text('route "/b"'), findsNothing); + + await tester.pumpWidget( + new MaterialApp( + initialRoute: '/b', + routes: routes, + ) + ); + expect(find.text('route "/"'), findsNothing); + expect(find.text('route "/a"'), findsOneWidget); + expect(find.text('route "/b"'), findsNothing); + + await tester.pumpWidget(new MaterialApp(routes: routes)); + expect(find.text('route "/"'), findsNothing); + expect(find.text('route "/a"'), findsOneWidget); + expect(find.text('route "/b"'), findsNothing); + }); }