From 9adb4a78a6c273f48b13614da0ceb220339c143f Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Fri, 23 Jun 2017 14:58:29 -0700 Subject: [PATCH] Deep linking: automatically push the route hiearchy on load. (#10894) The main purpose of this PR is to make it so that when you set the initial route and it's a hierarchical route (e.g. `/a/b/c`), it implies multiple pushes, one for each step of the route (so in that case, `/`, `/a`, `/a/b`, and `/a/b/c`, in that order). If any of those routes don't exist, it falls back to '/'. As part of doing that, I: * Changed the default for MaterialApp.initialRoute to honor the actual initial route. * Added a MaterialApp.onUnknownRoute for handling bad routes. * Added a feature to flutter_driver that allows the host test script and the device test app to communicate. * Added a test to make sure `flutter drive --route` works. (Hopefully that will also prove `flutter run --route` works, though this isn't testing the `flutter` tool's side of that. My main concern is over whether the engine side works.) * Fixed `flutter drive` to output the right target file name. * Changed how the stocks app represents its data, so that we can show a page for a stock before we know if it exists. * Made it possible to show a stock page that doesn't exist. It shows a progress indicator if we're loading the data, or else shows a message saying it doesn't exist. * Changed the pathing structure of routes in stocks to work more sanely. * Made search in the stocks app actually work (before it only worked if we happened to accidentally trigger a rebuild). Added a test. * Replaced some custom code in the stocks app with a BackButton. * Added a "color" feature to BackButton to support the stocks use case. * Spaced out the ErrorWidget text a bit more. * Added `RouteSettings.copyWith`, which I ended up not using. * Improved the error messages around routing. While I was in some files I made a few formatting fixes, fixed some code health issues, and also removed `flaky: true` from some devicelab tests that have been stable for a while. Also added some documentation here and there. --- .../lib/stocks/animation_bench.dart | 2 +- .../lib/stocks/build_bench.dart | 2 +- .../lib/stocks/layout_bench.dart | 2 +- dev/devicelab/bin/tasks/routing_test.dart | 85 ++++++++ dev/devicelab/lib/framework/framework.dart | 2 +- dev/devicelab/lib/framework/utils.dart | 9 +- dev/devicelab/lib/tasks/microbenchmarks.dart | 2 +- dev/devicelab/manifest.yaml | 8 +- dev/integration_tests/ui/README.md | 9 +- dev/integration_tests/ui/lib/main.dart | 2 +- dev/integration_tests/ui/lib/route.dart | 15 ++ .../ui/test_driver/keyboard_resize_test.dart | 3 +- .../ui/test_driver/route_test.dart | 22 ++ dev/manual_tests/lib/material_arc.dart | 2 +- examples/stocks/lib/main.dart | 37 ++-- examples/stocks/lib/stock_data.dart | 79 ++++--- examples/stocks/lib/stock_home.dart | 37 ++-- examples/stocks/lib/stock_symbol_viewer.dart | 54 +++-- examples/stocks/test/icon_color_test.dart | 4 +- examples/stocks/test/locale_test.dart | 6 +- examples/stocks/test/search_test.dart | 53 +++++ packages/flutter/lib/src/material/app.dart | 194 +++++++++++++++--- .../flutter/lib/src/material/back_button.dart | 9 +- packages/flutter/lib/src/rendering/error.dart | 2 +- packages/flutter/lib/src/widgets/app.dart | 56 ++++- packages/flutter/lib/src/widgets/basic.dart | 6 +- .../flutter/lib/src/widgets/navigator.dart | 127 ++++++++++-- packages/flutter/test/material/app_test.dart | 63 +++++- .../flutter/test/widgets/routes_test.dart | 6 + .../flutter_driver/lib/driver_extension.dart | 2 +- packages/flutter_driver/lib/src/driver.dart | 8 + .../flutter_driver/lib/src/extension.dart | 54 +++-- .../flutter_driver/lib/src/frame_sync.dart | 4 +- packages/flutter_driver/lib/src/health.dart | 9 +- .../flutter_driver/lib/src/request_data.dart | 46 +++++ .../flutter_driver/lib/src/semantics.dart | 4 +- .../test/src/extension_test.dart | 12 +- .../flutter_tools/lib/src/commands/drive.dart | 27 +-- .../flutter_tools/lib/src/commands/run.dart | 6 +- .../flutter_tools/lib/src/test/watcher.dart | 1 - 40 files changed, 861 insertions(+), 210 deletions(-) create mode 100644 dev/devicelab/bin/tasks/routing_test.dart create mode 100644 dev/integration_tests/ui/lib/route.dart create mode 100644 dev/integration_tests/ui/test_driver/route_test.dart create mode 100644 examples/stocks/test/search_test.dart create mode 100644 packages/flutter_driver/lib/src/request_data.dart diff --git a/dev/benchmarks/microbenchmarks/lib/stocks/animation_bench.dart b/dev/benchmarks/microbenchmarks/lib/stocks/animation_bench.dart index 0e1043bd10..8ebdad60f4 100644 --- a/dev/benchmarks/microbenchmarks/lib/stocks/animation_bench.dart +++ b/dev/benchmarks/microbenchmarks/lib/stocks/animation_bench.dart @@ -35,7 +35,7 @@ class BenchmarkingBinding extends LiveTestWidgetsFlutterBinding { Future main() async { assert(false); // don't run this in checked mode! Use --release. - stock_data.StockDataFetcher.actuallyFetchData = false; + stock_data.StockData.actuallyFetchData = false; final Stopwatch wallClockWatch = new Stopwatch(); final Stopwatch cpuWatch = new Stopwatch(); diff --git a/dev/benchmarks/microbenchmarks/lib/stocks/build_bench.dart b/dev/benchmarks/microbenchmarks/lib/stocks/build_bench.dart index dfde886a32..0e64828314 100644 --- a/dev/benchmarks/microbenchmarks/lib/stocks/build_bench.dart +++ b/dev/benchmarks/microbenchmarks/lib/stocks/build_bench.dart @@ -17,7 +17,7 @@ const Duration kBenchmarkTime = const Duration(seconds: 15); Future main() async { assert(false); // don't run this in checked mode! Use --release. - stock_data.StockDataFetcher.actuallyFetchData = false; + stock_data.StockData.actuallyFetchData = false; // This allows us to call onBeginFrame even when the engine didn't request it, // and have it actually do something: diff --git a/dev/benchmarks/microbenchmarks/lib/stocks/layout_bench.dart b/dev/benchmarks/microbenchmarks/lib/stocks/layout_bench.dart index 933ff41429..07eec5a9e8 100644 --- a/dev/benchmarks/microbenchmarks/lib/stocks/layout_bench.dart +++ b/dev/benchmarks/microbenchmarks/lib/stocks/layout_bench.dart @@ -16,7 +16,7 @@ import '../common.dart'; const Duration kBenchmarkTime = const Duration(seconds: 15); Future main() async { - stock_data.StockDataFetcher.actuallyFetchData = false; + stock_data.StockData.actuallyFetchData = false; // This allows us to call onBeginFrame even when the engine didn't request it, // and have it actually do something: diff --git a/dev/devicelab/bin/tasks/routing_test.dart b/dev/devicelab/bin/tasks/routing_test.dart new file mode 100644 index 0000000000..f7d8f4c78b --- /dev/null +++ b/dev/devicelab/bin/tasks/routing_test.dart @@ -0,0 +1,85 @@ +// Copyright (c) 2016 The Chromium 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 'dart:convert'; +import 'dart:io'; + +import 'package:path/path.dart' as path; + +import 'package:flutter_devicelab/framework/adb.dart'; +import 'package:flutter_devicelab/framework/framework.dart'; +import 'package:flutter_devicelab/framework/utils.dart'; + +void main() { + task(() async { + final Device device = await devices.workingDevice; + await device.unlock(); + final Directory appDir = dir(path.join(flutterDirectory.path, 'dev/integration_tests/ui')); + section('TEST WHETHER `flutter drive --route` WORKS'); + await inDirectory(appDir, () async { + return await flutter( + 'drive', + options: ['--verbose', '-d', device.deviceId, '--route', '/smuggle-it', 'lib/route.dart'], + canFail: false, + ); + }); + section('TEST WHETHER `flutter run --route` WORKS'); + await inDirectory(appDir, () async { + final Completer ready = new Completer(); + bool ok; + print('run: starting...'); + final Process run = await startProcess( + path.join(flutterDirectory.path, 'bin', 'flutter'), + ['run', '--verbose', '--observatory-port=8888', '-d', device.deviceId, '--route', '/smuggle-it', 'lib/route.dart'], + ); + run.stdout + .transform(UTF8.decoder) + .transform(const LineSplitter()) + .listen((String line) { + print('run:stdout: $line'); + if (line == '[ ] For a more detailed help message, press "h". To quit, press "q".') { + print('run: ready!'); + ready.complete(); + ok ??= true; + } + }); + run.stderr + .transform(UTF8.decoder) + .transform(const LineSplitter()) + .listen((String line) { + stderr.writeln('run:stderr: $line'); + }); + run.exitCode.then((int exitCode) { ok = false; }); + await Future.any(>[ ready.future, run.exitCode ]); + if (!ok) + throw 'Failed to run test app.'; + print('drive: starting...'); + final Process drive = await startProcess( + path.join(flutterDirectory.path, 'bin', 'flutter'), + ['drive', '--use-existing-app', 'http://127.0.0.1:8888/', '--no-keep-app-running', 'lib/route.dart'], + ); + drive.stdout + .transform(UTF8.decoder) + .transform(const LineSplitter()) + .listen((String line) { + print('drive:stdout: $line'); + }); + drive.stderr + .transform(UTF8.decoder) + .transform(const LineSplitter()) + .listen((String line) { + stderr.writeln('drive:stderr: $line'); + }); + int result; + result = await drive.exitCode; + if (result != 0) + throw 'Failed to drive test app (exit code $result).'; + result = await run.exitCode; + if (result != 0) + throw 'Received unexpected exit code $result from run process.'; + }); + return new TaskResult.success(null); + }); +} diff --git a/dev/devicelab/lib/framework/framework.dart b/dev/devicelab/lib/framework/framework.dart index 1d0cff0dc3..f86124eeee 100644 --- a/dev/devicelab/lib/framework/framework.dart +++ b/dev/devicelab/lib/framework/framework.dart @@ -26,7 +26,7 @@ bool _isTaskRegistered = false; /// Registers a [task] to run, returns the result when it is complete. /// -/// Note, the task does not run immediately but waits for the request via the +/// The task does not run immediately but waits for the request via the /// VM service protocol to run it. /// /// It is ok for a [task] to perform many things. However, only one task can be diff --git a/dev/devicelab/lib/framework/utils.dart b/dev/devicelab/lib/framework/utils.dart index 5b0c77e9d8..b3e837a25c 100644 --- a/dev/devicelab/lib/framework/utils.dart +++ b/dev/devicelab/lib/framework/utils.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'dart:math' as math; import 'package:args/args.dart'; import 'package:meta/meta.dart'; @@ -116,7 +117,12 @@ void mkdirs(Directory directory) { bool exists(FileSystemEntity entity) => entity.existsSync(); void section(String title) { - print('\n••• $title •••'); + title = '╡ ••• $title ••• ╞'; + final String line = '═' * math.max((80 - title.length) ~/ 2, 2); + String output = '$line$title$line'; + if (output.length == 79) + output += '═'; + print('\n\n$output\n'); } Future getDartVersion() async { @@ -179,6 +185,7 @@ Future startProcess( _runningProcesses.add(processInfo); process.exitCode.whenComplete(() { + print('\n'); // separate the output of this script from subsequent output to make logs easier to read _runningProcesses.remove(processInfo); }); diff --git a/dev/devicelab/lib/tasks/microbenchmarks.dart b/dev/devicelab/lib/tasks/microbenchmarks.dart index a3844145a3..a7ba367374 100644 --- a/dev/devicelab/lib/tasks/microbenchmarks.dart +++ b/dev/devicelab/lib/tasks/microbenchmarks.dart @@ -59,7 +59,7 @@ TaskFunction createMicrobenchmarkTask() { } Future _startFlutter({ - String command = 'run', + String command: 'run', List options: const [], bool canFail: false, Map environment, diff --git a/dev/devicelab/manifest.yaml b/dev/devicelab/manifest.yaml index 5eb0fd94a4..e698412248 100644 --- a/dev/devicelab/manifest.yaml +++ b/dev/devicelab/manifest.yaml @@ -119,13 +119,18 @@ tasks: Builds sample catalog markdown pages and Android screenshots stage: devicelab required_agent_capabilities: ["has-android-device"] - flaky: true complex_layout_semantics_perf: description: > Measures duration of building the initial semantics tree. stage: devicelab required_agent_capabilities: ["linux/android"] + + routing: + description: > + Verifies that `flutter drive --route` still works. No performance numbers. + stage: devicelab + required_agent_capabilities: ["linux/android"] flaky: true # iOS on-device tests @@ -277,7 +282,6 @@ tasks: with semantics enabled. stage: devicelab required_agent_capabilities: ["linux/android"] - flaky: true flutter_gallery__memory_nav: description: > diff --git a/dev/integration_tests/ui/README.md b/dev/integration_tests/ui/README.md index ff91ce7c10..364f593002 100644 --- a/dev/integration_tests/ui/README.md +++ b/dev/integration_tests/ui/README.md @@ -1,7 +1,14 @@ # Flutter UI integration tests -This project contains a collection of non-plugin-dependent UI integration tests. +This project contains a collection of non-plugin-dependent UI +integration tests. The device code is in the `lib/` directory, the +driver code is in the `test_driver/` directory. They work together. +Normally they are run via the devicelab. ## keyboard\_resize Verifies that showing and hiding the keyboard resizes the content. + +## routing + +Verifies that `flutter drive --route` works correctly. diff --git a/dev/integration_tests/ui/lib/main.dart b/dev/integration_tests/ui/lib/main.dart index 1e74ba0be1..66a92aad5f 100644 --- a/dev/integration_tests/ui/lib/main.dart +++ b/dev/integration_tests/ui/lib/main.dart @@ -4,4 +4,4 @@ import 'package:flutter/widgets.dart'; -void main() => runApp(const Center(child: const Text('flutter run -t xxx.dart'))); +void main() => runApp(const Center(child: const Text('flutter drive lib/xxx.dart'))); diff --git a/dev/integration_tests/ui/lib/route.dart b/dev/integration_tests/ui/lib/route.dart new file mode 100644 index 0000000000..2f8b507c58 --- /dev/null +++ b/dev/integration_tests/ui/lib/route.dart @@ -0,0 +1,15 @@ +// Copyright 2017 The Chromium 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:ui' as ui; + +import 'package:flutter_driver/driver_extension.dart'; + +// To use this test: "flutter drive --route '/smuggle-it' lib/route.dart" + +void main() { + enableFlutterDriverExtension(handler: (String message) async { + return ui.window.defaultRouteName; + }); +} diff --git a/dev/integration_tests/ui/test_driver/keyboard_resize_test.dart b/dev/integration_tests/ui/test_driver/keyboard_resize_test.dart index 5d624c60af..3efb7fc930 100644 --- a/dev/integration_tests/ui/test_driver/keyboard_resize_test.dart +++ b/dev/integration_tests/ui/test_driver/keyboard_resize_test.dart @@ -13,8 +13,7 @@ void main() { }); tearDownAll(() async { - if (driver != null) - driver.close(); + driver?.close(); }); test('Ensure keyboard dismissal resizes the view to original size', () async { diff --git a/dev/integration_tests/ui/test_driver/route_test.dart b/dev/integration_tests/ui/test_driver/route_test.dart new file mode 100644 index 0000000000..fa06384398 --- /dev/null +++ b/dev/integration_tests/ui/test_driver/route_test.dart @@ -0,0 +1,22 @@ +import 'package:flutter_driver/flutter_driver.dart'; +import 'package:test/test.dart'; + +void main() { + group('flutter run test --route', () { + FlutterDriver driver; + + setUpAll(() async { + driver = await FlutterDriver.connect(); + }); + + tearDownAll(() async { + driver?.close(); + }); + + test('sanity check flutter drive --route', () async { + // This only makes sense if you ran the test as described + // in the test file. It's normally run from devicelab. + expect(await driver.requestData('route'), '/smuggle-it'); + }); + }); +} diff --git a/dev/manual_tests/lib/material_arc.dart b/dev/manual_tests/lib/material_arc.dart index b6d71f671f..4846ab6eea 100644 --- a/dev/manual_tests/lib/material_arc.dart +++ b/dev/manual_tests/lib/material_arc.dart @@ -475,6 +475,6 @@ class _AnimationDemoState extends State with TickerProviderStateM void main() { runApp(new MaterialApp( - home: const AnimationDemo() + home: const AnimationDemo(), )); } diff --git a/examples/stocks/lib/main.dart b/examples/stocks/lib/main.dart index 1cbfd1a55a..46ed17d2dd 100644 --- a/examples/stocks/lib/main.dart +++ b/examples/stocks/lib/main.dart @@ -29,9 +29,7 @@ class StocksApp extends StatefulWidget { } class StocksAppState extends State { - - final Map _stocks = {}; - final List _symbols = []; + StockData stocks; StockConfiguration _configuration = new StockConfiguration( stockMode: StockMode.optimistic, @@ -49,11 +47,7 @@ class StocksAppState extends State { @override void initState() { super.initState(); - new StockDataFetcher((StockData data) { - setState(() { - data.appendTo(_stocks, _symbols); - }); - }); + stocks = new StockData(); } void configurationUpdater(StockConfiguration value) { @@ -80,19 +74,28 @@ class StocksAppState extends State { } Route _getRoute(RouteSettings settings) { + // Routes, by convention, are split on slashes, like filesystem paths. final List path = settings.name.split('/'); + // We only support paths that start with a slash, so bail if + // the first component is not empty: if (path[0] != '') return null; - if (path[1] == 'stock') { - if (path.length != 3) + // If the path is "/stock:..." then show a stock page for the + // specified stock symbol. + if (path[1].startsWith('stock:')) { + // We don't yet support subpages of a stock, so bail if there's + // any more path components. + if (path.length != 2) return null; - if (_stocks.containsKey(path[2])) { - return new MaterialPageRoute( - settings: settings, - builder: (BuildContext context) => new StockSymbolPage(stock: _stocks[path[2]]) - ); - } + // Extract the symbol part of "stock:..." and return a route + // for that symbol. + final String symbol = path[1].substring(6); + return new MaterialPageRoute( + settings: settings, + builder: (BuildContext context) => new StockSymbolPage(symbol: symbol, stocks: stocks), + ); } + // The other paths we support are in the routes table. return null; } @@ -120,7 +123,7 @@ class StocksAppState extends State { showPerformanceOverlay: _configuration.showPerformanceOverlay, showSemanticsDebugger: _configuration.showSemanticsDebugger, routes: { - '/': (BuildContext context) => new StockHome(_stocks, _symbols, _configuration, configurationUpdater), + '/': (BuildContext context) => new StockHome(stocks, _configuration, configurationUpdater), '/settings': (BuildContext context) => new StockSettings(_configuration, configurationUpdater) }, onGenerateRoute: _getRoute, diff --git a/examples/stocks/lib/stock_data.dart b/examples/stocks/lib/stock_data.dart index d84c37d305..57243b672f 100644 --- a/examples/stocks/lib/stock_data.dart +++ b/examples/stocks/lib/stock_data.dart @@ -10,6 +10,7 @@ import 'dart:convert'; import 'dart:math' as math; +import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; import 'package:http/http.dart' as http; @@ -38,54 +39,64 @@ class Stock { } } -class StockData { - StockData(this._data); - - final List> _data; - - void appendTo(Map stocks, List symbols) { - for (List fields in _data) { - final Stock stock = new Stock.fromFields(fields); - symbols.add(stock.symbol); - stocks[stock.symbol] = stock; +class StockData extends ChangeNotifier { + StockData() { + if (actuallyFetchData) { + _httpClient = createHttpClient(); + _fetchNextChunk(); } - symbols.sort(); - } -} - -typedef void StockDataCallback(StockData data); -const int _kChunkCount = 30; - -String _urlToFetch(int chunk) { - return 'https://domokit.github.io/examples/stocks/data/stock_data_$chunk.json'; -} - -class StockDataFetcher { - StockDataFetcher(this.callback) { - _httpClient = createHttpClient(); - _fetchNextChunk(); } - final StockDataCallback callback; + final List _symbols = []; + final Map _stocks = {}; + + Iterable get allSymbols => _symbols; + + Stock operator [](String symbol) => _stocks[symbol]; + + bool get loading => _httpClient != null; + + void add(List> data) { + for (List fields in data) { + final Stock stock = new Stock.fromFields(fields); + _symbols.add(stock.symbol); + _stocks[stock.symbol] = stock; + } + _symbols.sort(); + notifyListeners(); + } + + static const int _kChunkCount = 30; + int _nextChunk = 0; + + String _urlToFetch(int chunk) { + return 'https://domokit.github.io/examples/stocks/data/stock_data_$chunk.json'; + } + http.Client _httpClient; static bool actuallyFetchData = true; - int _nextChunk = 0; - void _fetchNextChunk() { - if (!actuallyFetchData) - return; _httpClient.get(_urlToFetch(_nextChunk++)).then((http.Response response) { final String json = response.body; if (json == null) { - print("Failed to load stock data chunk ${_nextChunk - 1}"); - return null; + debugPrint('Failed to load stock data chunk ${_nextChunk - 1}'); + _end(); + return; } final JsonDecoder decoder = const JsonDecoder(); - callback(new StockData(decoder.convert(json))); - if (_nextChunk < _kChunkCount) + add(decoder.convert(json)); + if (_nextChunk < _kChunkCount) { _fetchNextChunk(); + } else { + _end(); + } }); } + + void _end() { + _httpClient?.close(); + _httpClient = null; + } } diff --git a/examples/stocks/lib/stock_home.dart b/examples/stocks/lib/stock_home.dart index 1066cad088..340ba05686 100644 --- a/examples/stocks/lib/stock_home.dart +++ b/examples/stocks/lib/stock_home.dart @@ -50,10 +50,9 @@ class _NotImplementedDialog extends StatelessWidget { } class StockHome extends StatefulWidget { - const StockHome(this.stocks, this.symbols, this.configuration, this.updater); + const StockHome(this.stocks, this.configuration, this.updater); - final Map stocks; - final List symbols; + final StockData stocks; final StockConfiguration configuration; final ValueChanged updater; @@ -62,10 +61,9 @@ class StockHome extends StatefulWidget { } class StockHomeState extends State { - final GlobalKey _scaffoldKey = new GlobalKey(); - bool _isSearching = false; final TextEditingController _searchQuery = new TextEditingController(); + bool _isSearching = false; bool _autorefresh = false; void _handleSearchBegin() { @@ -82,10 +80,6 @@ class StockHomeState extends State { }); } - void _handleSearchEnd() { - Navigator.pop(context); - } - void _handleStockModeChange(StockMode value) { if (widget.updater != null) widget.updater(widget.configuration.copyWith(stockMode: value)); @@ -233,8 +227,8 @@ class StockHomeState extends State { ); } - Iterable _getStockList(Iterable symbols) { - return symbols.map((String symbol) => widget.stocks[symbol]) + static Iterable _getStockList(StockData stocks, Iterable symbols) { + return symbols.map((String symbol) => stocks[symbol]) .where((Stock stock) => stock != null); } @@ -266,7 +260,7 @@ class StockHomeState extends State { stocks: stocks.toList(), onAction: _buyStock, onOpen: (Stock stock) { - Navigator.pushNamed(context, '/stock/${stock.symbol}'); + Navigator.pushNamed(context, '/stock:${stock.symbol}'); }, onShow: (Stock stock) { _scaffoldKey.currentState.showBottomSheet((BuildContext context) => new StockSymbolBottomSheet(stock: stock)); @@ -275,22 +269,21 @@ class StockHomeState extends State { } Widget _buildStockTab(BuildContext context, StockHomeTab tab, List stockSymbols) { - return new Container( + return new AnimatedBuilder( key: new ValueKey(tab), - child: _buildStockList(context, _filterBySearchQuery(_getStockList(stockSymbols)).toList(), tab), + animation: new Listenable.merge([_searchQuery, widget.stocks]), + builder: (BuildContext context, Widget child) { + return _buildStockList(context, _filterBySearchQuery(_getStockList(widget.stocks, stockSymbols)).toList(), tab); + }, ); } static const List portfolioSymbols = const ["AAPL","FIZZ", "FIVE", "FLAT", "ZINC", "ZNGA"]; - // TODO(abarth): Should we factor this into a SearchBar in the framework? Widget buildSearchBar() { return new AppBar( - leading: new IconButton( - icon: const Icon(Icons.arrow_back), + leading: new BackButton( color: Theme.of(context).accentColor, - onPressed: _handleSearchEnd, - tooltip: 'Back', ), title: new TextField( controller: _searchQuery, @@ -330,7 +323,7 @@ class StockHomeState extends State { drawer: _buildDrawer(context), body: new TabBarView( children: [ - _buildStockTab(context, StockHomeTab.market, widget.symbols), + _buildStockTab(context, StockHomeTab.market, widget.stocks.allSymbols), _buildStockTab(context, StockHomeTab.portfolio, portfolioSymbols), ], ), @@ -342,7 +335,6 @@ class StockHomeState extends State { class _CreateCompanySheet extends StatelessWidget { @override Widget build(BuildContext context) { - // TODO(ianh): Fill this out. return new Column( children: [ const TextField( @@ -351,6 +343,9 @@ class _CreateCompanySheet extends StatelessWidget { hintText: 'Company Name', ), ), + const Text('(This demo is not yet complete.)'), + // For example, we could add a button that actually updates the list + // and then contacts the server, etc. ], ); } diff --git a/examples/stocks/lib/stock_symbol_viewer.dart b/examples/stocks/lib/stock_symbol_viewer.dart index 6d299196b8..27095ce751 100644 --- a/examples/stocks/lib/stock_symbol_viewer.dart +++ b/examples/stocks/lib/stock_symbol_viewer.dart @@ -15,6 +15,7 @@ class _StockSymbolView extends StatelessWidget { @override Widget build(BuildContext context) { + assert(stock != null); final String lastSale = "\$${stock.lastSale.toStringAsFixed(2)}"; String changeInPrice = "${stock.percentChange.toStringAsFixed(2)}%"; if (stock.percentChange > 0) @@ -63,30 +64,49 @@ class _StockSymbolView extends StatelessWidget { } class StockSymbolPage extends StatelessWidget { - const StockSymbolPage({ this.stock }); + const StockSymbolPage({ this.symbol, this.stocks }); - final Stock stock; + final String symbol; + final StockData stocks; @override Widget build(BuildContext context) { - return new Scaffold( - appBar: new AppBar( - title: new Text(stock.name) - ), - body: new SingleChildScrollView( - child: new Container( - margin: const EdgeInsets.all(20.0), - child: new Card( - child: new _StockSymbolView( - stock: stock, - arrow: new Hero( - tag: stock, - child: new StockArrow(percentChange: stock.percentChange) + return new AnimatedBuilder( + animation: stocks, + builder: (BuildContext context, Widget child) { + final Stock stock = stocks[symbol]; + return new Scaffold( + appBar: new AppBar( + title: new Text(stock?.name ?? symbol) + ), + body: new SingleChildScrollView( + child: new Container( + margin: const EdgeInsets.all(20.0), + child: new Card( + child: new AnimatedCrossFade( + duration: const Duration(milliseconds: 300), + firstChild: const Padding( + padding: const EdgeInsets.all(20.0), + child: const Center(child: const CircularProgressIndicator()), + ), + secondChild: stock != null + ? new _StockSymbolView( + stock: stock, + arrow: new Hero( + tag: stock, + child: new StockArrow(percentChange: stock.percentChange), + ), + ) : new Padding( + padding: const EdgeInsets.all(20.0), + child: new Center(child: new Text('$symbol not found')), + ), + crossFadeState: stock == null && stocks.loading ? CrossFadeState.showFirst : CrossFadeState.showSecond, + ), ) ) ) - ) - ) + ); + }, ); } } diff --git a/examples/stocks/test/icon_color_test.dart b/examples/stocks/test/icon_color_test.dart index e2dddd6dbe..6d2018c0eb 100644 --- a/examples/stocks/test/icon_color_test.dart +++ b/examples/stocks/test/icon_color_test.dart @@ -46,9 +46,9 @@ void checkIconColor(WidgetTester tester, String label, Color color) { } void main() { - stock_data.StockDataFetcher.actuallyFetchData = false; + stock_data.StockData.actuallyFetchData = false; - testWidgets("Test icon colors", (WidgetTester tester) async { + testWidgets('Icon colors', (WidgetTester tester) async { stocks.main(); // builds the app and schedules a frame but doesn't trigger one await tester.pump(); // see https://github.com/flutter/flutter/issues/1865 await tester.pump(); // triggers a frame diff --git a/examples/stocks/test/locale_test.dart b/examples/stocks/test/locale_test.dart index 2dc30ecf0b..e9e6b5d378 100644 --- a/examples/stocks/test/locale_test.dart +++ b/examples/stocks/test/locale_test.dart @@ -7,14 +7,14 @@ import 'package:stocks/main.dart' as stocks; import 'package:stocks/stock_data.dart' as stock_data; void main() { - stock_data.StockDataFetcher.actuallyFetchData = false; + stock_data.StockData.actuallyFetchData = false; - testWidgets("Test changing locale", (WidgetTester tester) async { + testWidgets('Changing locale', (WidgetTester tester) async { stocks.main(); await tester.idle(); // see https://github.com/flutter/flutter/issues/1865 await tester.pump(); expect(find.text('MARKET'), findsOneWidget); - await tester.binding.setLocale("es", "US"); + await tester.binding.setLocale('es', 'US'); await tester.idle(); await tester.pump(); expect(find.text('MERCADO'), findsOneWidget); diff --git a/examples/stocks/test/search_test.dart b/examples/stocks/test/search_test.dart new file mode 100644 index 0000000000..336327a1a4 --- /dev/null +++ b/examples/stocks/test/search_test.dart @@ -0,0 +1,53 @@ +// Copyright 2016 The Chromium 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 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:stocks/main.dart' as stocks; +import 'package:stocks/stock_data.dart' as stock_data; + +void main() { + stock_data.StockData.actuallyFetchData = false; + + testWidgets('Search', (WidgetTester tester) async { + stocks.main(); // builds the app and schedules a frame but doesn't trigger one + await tester.pump(); // see https://github.com/flutter/flutter/issues/1865 + await tester.pump(); // triggers a frame + + expect(find.text('AAPL'), findsNothing); + expect(find.text('BANA'), findsNothing); + + final stocks.StocksAppState app = tester.state(find.byType(stocks.StocksApp)); + app.stocks.add(>[ + // "Symbol","Name","LastSale","MarketCap","IPOyear","Sector","industry","Summary Quote" + ['AAPL', 'Apple', '', '', '', '', '', ''], + ['BANA', 'Banana', '', '', '', '', '', ''], + ]); + await tester.pump(); + + expect(find.text('AAPL'), findsOneWidget); + expect(find.text('BANA'), findsOneWidget); + + await tester.tap(find.byTooltip('Search')); + // We skip a minute at a time so that each phase of the animation + // is done in two frames, the start frame and the end frame. + // There are two phases currently, so that results in three frames. + expect(await tester.pumpAndSettle(const Duration(minutes: 1)), 3); + + expect(find.text('AAPL'), findsOneWidget); + expect(find.text('BANA'), findsOneWidget); + + await tester.enterText(find.byType(EditableText), 'B'); + await tester.pump(); + + expect(find.text('AAPL'), findsNothing); + expect(find.text('BANA'), findsOneWidget); + + await tester.enterText(find.byType(EditableText), 'X'); + await tester.pump(); + + expect(find.text('AAPL'), findsNothing); + expect(find.text('BANA'), findsNothing); + }); +} diff --git a/packages/flutter/lib/src/material/app.dart b/packages/flutter/lib/src/material/app.dart index a860860d86..8b035d443e 100644 --- a/packages/flutter/lib/src/material/app.dart +++ b/packages/flutter/lib/src/material/app.dart @@ -26,10 +26,29 @@ const TextStyle _errorTextStyle = const TextStyle( /// An application that uses material design. /// /// A convenience widget that wraps a number of widgets that are commonly -/// required for material design applications. It builds upon a -/// [WidgetsApp] by adding material-design specific functionality, such as -/// [AnimatedTheme] and [GridPaper]. This widget also configures the top-level -/// [Navigator]'s observer to perform [Hero] animations. +/// required for material design applications. It builds upon a [WidgetsApp] by +/// adding material-design specific functionality, such as [AnimatedTheme] and +/// [GridPaper]. +/// +/// The [MaterialApp] configures the top-level [Navigator] to search for routes +/// in the following order: +/// +/// 1. For the `/` route, the [home] property, if non-null, is used. +/// +/// 2. Otherwise, the [routes] table is used, if it has an entry for the route. +/// +/// 3. Otherwise, [onGenerateRoute] is called, if provided. It should return a +/// non-null value for any _valid_ route not handled by [home] and [routes]. +/// +/// 4. Finally if all else fails [onUnknownRoute] is called. +/// +/// At least one of these options must handle the `/` route, since it is used +/// when an invalid [initialRoute] is specified on startup (e.g. by another +/// application launching this one with an intent on Android; see +/// [Window.defaultRouteName]). +/// +/// This widget also configures the top-level [Navigator]'s observer to perform +/// [Hero] animations. /// /// See also: /// @@ -38,24 +57,27 @@ const TextStyle _errorTextStyle = const TextStyle( /// * [MaterialPageRoute], which defines an app page that transitions in a material-specific way. /// * [WidgetsApp], which defines the basic app elements but does not depend on the material library. 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 - /// [initialRoute], which defaults to [Navigator.defaultRouteName] - /// (`'/'`). + /// 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] (`/`), since that is the route used when the + /// application is launched with an intent that specifies an otherwise + /// unsupported route. /// /// This class creates an instance of [WidgetsApp]. - MaterialApp({ + /// + /// The boolean arguments, [routes], and [navigatorObservers], must not be null. + MaterialApp({ // can't be const because the asserts use methods on Map :-( Key key, this.title, this.color, this.theme, this.home, this.routes: const {}, - this.initialRoute: Navigator.defaultRouteName, + this.initialRoute, this.onGenerateRoute, + this.onUnknownRoute, this.onLocaleChanged, this.navigatorObservers: const [], this.debugShowMaterialGrid: false, @@ -64,10 +86,32 @@ class MaterialApp extends StatefulWidget { this.checkerboardOffscreenLayers: false, this.showSemanticsDebugger: false, this.debugShowCheckedModeBanner: true - }) : assert(debugShowMaterialGrid != null), - assert(routes != null), - assert(!routes.containsKey(initialRoute) || (home == null)), - assert(routes.containsKey(initialRoute) || (home != null) || (onGenerateRoute != null)), + }) : assert(routes != null), + assert(navigatorObservers != null), + assert(debugShowMaterialGrid != null), + assert(showPerformanceOverlay != null), + assert(checkerboardRasterCacheImages != null), + assert(checkerboardOffscreenLayers != null), + assert(showSemanticsDebugger != null), + assert(debugShowCheckedModeBanner != null), + assert( + home == null || + !routes.containsKey(Navigator.defaultRouteName), + 'If the home property is specified, the routes table ' + 'cannot include an entry for "/", since it would be redundant.' + ), + assert( + home != null || + routes.containsKey(Navigator.defaultRouteName) || + onGenerateRoute != null || + onUnknownRoute != null, + 'Either the home property must be specified, ' + 'or the routes table must include an entry for "/", ' + 'or there must be on onGenerateRoute callback specified, ' + 'or there must be an onUnknownRoute callback specified, ' + 'because otherwise there is nothing to fall back on if the ' + 'app is started with an intent that specifies an unknown route.' + ), super(key: key); /// A one-line description of this app for use in the window manager. @@ -76,20 +120,20 @@ class MaterialApp extends StatefulWidget { /// The colors to use for the application's widgets. final ThemeData theme; - /// The widget for the default route of the app - /// ([Navigator.defaultRouteName], which is `'/'`). + /// The widget for the default route of the app ([Navigator.defaultRouteName], + /// which is `/`). /// - /// This is the page that is displayed first when the application is - /// started normally. + /// This is the route that is displayed first when the application is started + /// normally, unless [initialRoute] is specified. It's also the route that's + /// displayed if the [initialRoute] can't be displayed. /// /// To be able to directly call [Theme.of], [MediaQuery.of], /// [LocaleQuery.of], etc, in the code sets the [home] argument in /// the constructor, you can use a [Builder] widget to get a /// [BuildContext]. /// - /// If this is not specified, then either the route with name `'/'` - /// must be given in [routes], or the [onGenerateRoute] callback - /// must be able to build a widget for that route. + /// If [home] is specified, then [routes] must not include an entry for `/`, + /// as [home] takes its place. final Widget home; /// The primary color to use for the application in the operating system @@ -108,29 +152,71 @@ class MaterialApp extends StatefulWidget { /// /// If the app only has one page, then you can specify it using [home] instead. /// - /// If [home] is specified, then it is an error to provide a route - /// in this map for the [Navigator.defaultRouteName] route (`'/'`). + /// If [home] is specified, then it implies an entry in this table for the + /// [Navigator.defaultRouteName] route (`/`), and it is an error to + /// redundantly provide such a route in the [routes] table. /// - /// If a route is requested that is not specified in this table (or - /// by [home]), then the [onGenerateRoute] callback is called to - /// build the page instead. + /// If a route is requested that is not specified in this table (or by + /// [home]), then the [onGenerateRoute] callback is called to build the page + /// instead. final Map routes; /// The name of the first route to show. /// - /// Defaults to [Window.defaultRouteName]. + /// Defaults to [Window.defaultRouteName], which may be overridden by the code + /// that launched the application. + /// + /// If the route contains slashes, then it is treated as a "deep link", and + /// before this route is pushed, the routes leading to this one are pushed + /// also. For example, if the route was `/a/b/c`, then the app would start + /// with the three routes `/a`, `/a/b`, and `/a/b/c` loaded, in that order. + /// + /// If any part of this process fails to generate routes, then the + /// [initialRoute] is ignored and [Navigator.defaultRouteName] is used instead + /// (`/`). This can happen if the app is started with an intent that specifies + /// a non-existent route. + /// + /// See also: + /// + /// * [Navigator.initialRoute], which is used to implement this property. + /// * [Navigator.push], for pushing additional routes. + /// * [Navigator.pop], for removing a route from the stack. final String initialRoute; /// The route generator callback used when the app is navigated to a /// named route. + /// + /// This is used if [routes] does not contain the requested route. + /// + /// If this returns null when building the routes to handle the specified + /// [initialRoute], then all the routes are discarded and + /// [Navigator.defaultRouteName] is used instead (`/`). See [initialRoute]. + /// + /// During normal app operation, the [onGenerateRoute] callback will only be + /// applied to route names pushed by the application, and so should never + /// return null. final RouteFactory onGenerateRoute; + /// Called when [onGenerateRoute] fails to generate a route, except for the + /// [initialRoute]. + /// + /// This callback is typically used for error handling. For example, this + /// callback might always generate a "not found" page that describes the route + /// that wasn't found. + /// + /// The default implementation pushes a route that displays an ugly error + /// message. + final RouteFactory onUnknownRoute; + /// Callback that is called when the operating system changes the /// current locale. final LocaleChangedCallback onLocaleChanged; /// Turns on a performance overlay. - /// https://flutter.io/debugging/#performanceoverlay + /// + /// See also: + /// + /// * final bool showPerformanceOverlay; /// Turns on checkerboarding of raster cache images. @@ -162,9 +248,13 @@ class MaterialApp extends StatefulWidget { final List navigatorObservers; /// Turns on a [GridPaper] overlay that paints a baseline grid - /// Material apps: - /// https://material.google.com/layout/metrics-keylines.html + /// Material apps. + /// /// Only available in checked mode. + /// + /// See also: + /// + /// * final bool debugShowMaterialGrid; @override @@ -210,13 +300,16 @@ class _MaterialAppState extends State { } Route _onGenerateRoute(RouteSettings settings) { - WidgetBuilder builder = widget.routes[settings.name]; - if (builder == null && widget.home != null && settings.name == Navigator.defaultRouteName) + final String name = settings.name; + WidgetBuilder builder; + if (name == Navigator.defaultRouteName && widget.home != null) builder = (BuildContext context) => widget.home; + else + builder = widget.routes[name]; if (builder != null) { return new MaterialPageRoute( builder: builder, - settings: settings + settings: settings, ); } if (widget.onGenerateRoute != null) @@ -224,6 +317,38 @@ class _MaterialAppState extends State { return null; } + Route _onUnknownRoute(RouteSettings settings) { + assert(() { + if (widget.onUnknownRoute == null) { + throw new FlutterError( + 'Could not find a generator for route $settings in the $runtimeType.\n' + 'Generators for routes are searched for in the following order:\n' + ' 1. For the "/" route, the "home" property, if non-null, is used.\n' + ' 2. Otherwise, the "routes" table is used, if it has an entry for ' + 'the route.\n' + ' 3. Otherwise, onGenerateRoute is called. It should return a ' + 'non-null value for any valid route not handled by "home" and "routes".\n' + ' 4. Finally if all else fails onUnknownRoute is called.\n' + 'Unfortunately, onUnknownRoute was not set.' + ); + } + return true; + }); + final Route result = widget.onUnknownRoute(settings); + assert(() { + if (result == null) { + throw new FlutterError( + 'The onUnknownRoute callback returned null.\n' + 'When the $runtimeType requested the route $settings from its ' + 'onUnknownRoute callback, the callback returned null. Such callbacks ' + 'must never return null.' + ); + } + return true; + }); + return result; + } + @override Widget build(BuildContext context) { final ThemeData theme = widget.theme ?? new ThemeData.fallback(); @@ -241,6 +366,7 @@ class _MaterialAppState extends State { ..add(_heroController), initialRoute: widget.initialRoute, onGenerateRoute: _onGenerateRoute, + onUnknownRoute: _onUnknownRoute, onLocaleChanged: widget.onLocaleChanged, showPerformanceOverlay: widget.showPerformanceOverlay, checkerboardRasterCacheImages: widget.checkerboardRasterCacheImages, diff --git a/packages/flutter/lib/src/material/back_button.dart b/packages/flutter/lib/src/material/back_button.dart index 671bfeeb87..57977c4f1f 100644 --- a/packages/flutter/lib/src/material/back_button.dart +++ b/packages/flutter/lib/src/material/back_button.dart @@ -69,12 +69,19 @@ class BackButtonIcon extends StatelessWidget { class BackButton extends StatelessWidget { /// Creates an [IconButton] with the appropriate "back" icon for the current /// target platform. - const BackButton({ Key key }) : super(key: key); + const BackButton({ Key key, this.color }) : super(key: key); + + /// The color to use for the icon. + /// + /// Defaults to the [IconThemeData.color] specified in the ambient [IconTheme], + /// which usually matches the ambient [Theme]'s [ThemeData.iconTheme]. + final Color color; @override Widget build(BuildContext context) { return new IconButton( icon: const BackButtonIcon(), + color: color, tooltip: 'Back', // TODO(ianh): Figure out how to localize this string onPressed: () { Navigator.of(context).maybePop(); diff --git a/packages/flutter/lib/src/rendering/error.dart b/packages/flutter/lib/src/rendering/error.dart index ddd0af8117..7dfda9c001 100644 --- a/packages/flutter/lib/src/rendering/error.dart +++ b/packages/flutter/lib/src/rendering/error.dart @@ -93,7 +93,7 @@ class RenderErrorBox extends RenderBox { /// The paragraph style to use when painting [RenderErrorBox] objects. static ui.ParagraphStyle paragraphStyle = new ui.ParagraphStyle( - lineHeight: 0.85 + lineHeight: 1.0, ); @override diff --git a/packages/flutter/lib/src/widgets/app.dart b/packages/flutter/lib/src/widgets/app.dart index 669716d9c1..66faacb6b9 100644 --- a/packages/flutter/lib/src/widgets/app.dart +++ b/packages/flutter/lib/src/widgets/app.dart @@ -38,9 +38,13 @@ typedef Future LocaleChangedCallback(Locale locale); class WidgetsApp extends StatefulWidget { /// Creates a widget that wraps a number of widgets that are commonly /// required for an application. + /// + /// The boolean arguments, [color], [navigatorObservers], and + /// [onGenerateRoute] must not be null. const WidgetsApp({ Key key, @required this.onGenerateRoute, + this.onUnknownRoute, this.title, this.textStyle, @required this.color, @@ -52,12 +56,14 @@ class WidgetsApp extends StatefulWidget { this.checkerboardOffscreenLayers: false, this.showSemanticsDebugger: false, this.debugShowCheckedModeBanner: true - }) : assert(color != null), - assert(onGenerateRoute != null), + }) : assert(onGenerateRoute != null), + assert(color != null), + assert(navigatorObservers != null), assert(showPerformanceOverlay != null), assert(checkerboardRasterCacheImages != null), assert(checkerboardOffscreenLayers != null), assert(showSemanticsDebugger != null), + assert(debugShowCheckedModeBanner != null), super(key: key); /// A one-line description of this app for use in the window manager. @@ -75,11 +81,46 @@ class WidgetsApp extends StatefulWidget { /// The route generator callback used when the app is navigated to a /// named route. + /// + /// If this returns null when building the routes to handle the specified + /// [initialRoute], then all the routes are discarded and + /// [Navigator.defaultRouteName] is used instead (`/`). See [initialRoute]. + /// + /// During normal app operation, the [onGenerateRoute] callback will only be + /// applied to route names pushed by the application, and so should never + /// return null. final RouteFactory onGenerateRoute; + /// Called when [onGenerateRoute] fails to generate a route. + /// + /// This callback is typically used for error handling. For example, this + /// callback might always generate a "not found" page that describes the route + /// that wasn't found. + /// + /// Unknown routes can arise either from errors in the app or from external + /// requests to push routes, such as from Android intents. + final RouteFactory onUnknownRoute; + /// The name of the first route to show. /// - /// Defaults to [Window.defaultRouteName]. + /// Defaults to [Window.defaultRouteName], which may be overridden by the code + /// that launched the application. + /// + /// If the route contains slashes, then it is treated as a "deep link", and + /// before this route is pushed, the routes leading to this one are pushed + /// also. For example, if the route was `/a/b/c`, then the app would start + /// with the three routes `/a`, `/a/b`, and `/a/b/c` loaded, in that order. + /// + /// If any part of this process fails to generate routes, then the + /// [initialRoute] is ignored and [Navigator.defaultRouteName] is used instead + /// (`/`). This can happen if the app is started with an intent that specifies + /// a non-existent route. + /// + /// See also: + /// + /// * [Navigator.initialRoute], which is used to implement this property. + /// * [Navigator.push], for pushing additional routes. + /// * [Navigator.pop], for removing a route from the stack. final String initialRoute; /// Callback that is called when the operating system changes the @@ -221,6 +262,7 @@ class _WidgetsAppState extends State implements WidgetsBindingObserv key: _navigator, initialRoute: widget.initialRoute ?? ui.window.defaultRouteName, onGenerateRoute: widget.onGenerateRoute, + onUnknownRoute: widget.onUnknownRoute, observers: widget.navigatorObservers ) ) @@ -238,13 +280,13 @@ class _WidgetsAppState extends State implements WidgetsBindingObserv // options are set. if (widget.showPerformanceOverlay || WidgetsApp.showPerformanceOverlayOverride) { performanceOverlay = new PerformanceOverlay.allEnabled( - checkerboardRasterCacheImages: widget.checkerboardRasterCacheImages, - checkerboardOffscreenLayers: widget.checkerboardOffscreenLayers, + checkerboardRasterCacheImages: widget.checkerboardRasterCacheImages, + checkerboardOffscreenLayers: widget.checkerboardOffscreenLayers, ); } else if (widget.checkerboardRasterCacheImages || widget.checkerboardOffscreenLayers) { performanceOverlay = new PerformanceOverlay( - checkerboardRasterCacheImages: widget.checkerboardRasterCacheImages, - checkerboardOffscreenLayers: widget.checkerboardOffscreenLayers, + checkerboardRasterCacheImages: widget.checkerboardRasterCacheImages, + checkerboardOffscreenLayers: widget.checkerboardOffscreenLayers, ); } diff --git a/packages/flutter/lib/src/widgets/basic.dart b/packages/flutter/lib/src/widgets/basic.dart index 6444b413d9..372a845a63 100644 --- a/packages/flutter/lib/src/widgets/basic.dart +++ b/packages/flutter/lib/src/widgets/basic.dart @@ -12,7 +12,11 @@ import 'debug.dart'; import 'framework.dart'; export 'package:flutter/animation.dart'; -export 'package:flutter/foundation.dart' show TargetPlatform; +export 'package:flutter/foundation.dart' show + ChangeNotifier, + Listenable, + TargetPlatform, + ValueNotifier; export 'package:flutter/painting.dart'; export 'package:flutter/rendering.dart' show Axis, diff --git a/packages/flutter/lib/src/widgets/navigator.dart b/packages/flutter/lib/src/widgets/navigator.dart index 94ab3b5ebd..6cc8e4499f 100644 --- a/packages/flutter/lib/src/widgets/navigator.dart +++ b/packages/flutter/lib/src/widgets/navigator.dart @@ -207,6 +207,18 @@ class RouteSettings { this.isInitialRoute: false, }); + /// Creates a copy of this route settings object with the given fields + /// replaced with the new values. + RouteSettings copyWith({ + String name, + bool isInitialRoute, + }) { + return new RouteSettings( + name: name ?? this.name, + isInitialRoute: isInitialRoute ?? this.isInitialRoute, + ); + } + /// The name of the route (e.g., "/settings"). /// /// If null, the route is anonymous. @@ -374,7 +386,7 @@ typedef bool RoutePredicate(Route route); /// The app's home page route is named '/' by default. /// /// The [MaterialApp] can be created -/// with a `Map` which maps from a route's name to +/// with a [Map] which maps from a route's name to /// a builder function that will create it. The [MaterialApp] uses this /// map to create a value for its navigator's [onGenerateRoute] callback. /// @@ -496,6 +508,17 @@ class Navigator extends StatefulWidget { super(key: key); /// The name of the first route to show. + /// + /// By default, this defers to [dart:ui.Window.defaultRouteName]. + /// + /// If this string contains any `/` characters, then the string is split on + /// those characters and substrings from the start of the string up to each + /// such character are, in turn, used as routes to push. + /// + /// For example, if the route `/stocks/HOOLI` was used as the [initialRoute], + /// then the [Navigator] would push the following routes on startup: `/`, + /// `/stocks`, `/stocks/HOOLI`. This enables deep linking while allowing the + /// application to maintain a predictable route history. final String initialRoute; /// Called to generate a route for a given [RouteSettings]. @@ -514,7 +537,12 @@ class Navigator extends StatefulWidget { /// A list of observers for this navigator. final List observers; - /// The default name for the initial route. + /// The default name for the [initialRoute]. + /// + /// See also: + /// + /// * [dart:ui.Window.defaultRouteName], which reflects the route that the + /// application was started with. static const String defaultRouteName = '/'; /// Push a named route onto the navigator that most tightly encloses the given context. @@ -730,6 +758,8 @@ class NavigatorState extends State with TickerProviderStateMixin { /// The [FocusScopeNode] for the [FocusScope] that encloses the routes. final FocusScopeNode focusScopeNode = new FocusScopeNode(); + final List _initialOverlayEntries = []; + @override void initState() { super.initState(); @@ -737,10 +767,57 @@ class NavigatorState extends State with TickerProviderStateMixin { assert(observer.navigator == null); observer._navigator = this; } - push(widget.onGenerateRoute(new RouteSettings( - name: widget.initialRoute ?? Navigator.defaultRouteName, - isInitialRoute: true - ))); + String initialRouteName = widget.initialRoute ?? Navigator.defaultRouteName; + if (initialRouteName.startsWith('/') && initialRouteName.length > 1) { + initialRouteName = initialRouteName.substring(1); // strip leading '/' + assert(Navigator.defaultRouteName == '/'); + final List plannedInitialRouteNames = [ + Navigator.defaultRouteName, + ]; + final List> plannedInitialRoutes = >[ + _routeNamed(Navigator.defaultRouteName, allowNull: true), + ]; + final List routeParts = initialRouteName.split('/'); + if (initialRouteName.isNotEmpty) { + String routeName = ''; + for (String part in routeParts) { + routeName += '/$part'; + plannedInitialRouteNames.add(routeName); + plannedInitialRoutes.add(_routeNamed(routeName, allowNull: true)); + } + } + if (plannedInitialRoutes.contains(null)) { + assert(() { + FlutterError.reportError( + new FlutterErrorDetails( // ignore: prefer_const_constructors, https://github.com/dart-lang/sdk/issues/29952 + exception: + 'Could not navigate to initial route.\n' + 'The requested route name was: "/$initialRouteName"\n' + 'The following routes were therefore attempted:\n' + ' * ${plannedInitialRouteNames.join("\n * ")}\n' + 'This resulted in the following objects:\n' + ' * ${plannedInitialRoutes.join("\n * ")}\n' + 'One or more of those objects was null, and therefore the initial route specified will be ' + 'ignored and "${Navigator.defaultRouteName}" will be used instead.' + ), + ); + return true; + }); + push(_routeNamed(Navigator.defaultRouteName)); + } else { + for (Route route in plannedInitialRoutes) + push(route); + } + } else { + Route route; + if (initialRouteName != Navigator.defaultRouteName) + route = _routeNamed(initialRouteName, allowNull: true); + if (route == null) + route = _routeNamed(Navigator.defaultRouteName); + push(route); + } + for (Route route in _history) + _initialOverlayEntries.addAll(route.overlayEntries); } @override @@ -785,15 +862,40 @@ class NavigatorState extends State with TickerProviderStateMixin { bool _debugLocked = false; // used to prevent re-entrant calls to push, pop, and friends - Route _routeNamed(String name) { + Route _routeNamed(String name, { bool allowNull: false }) { assert(!_debugLocked); assert(name != null); - final RouteSettings settings = new RouteSettings(name: name); + final RouteSettings settings = new RouteSettings( + name: name, + isInitialRoute: _history.isEmpty, + ); Route route = widget.onGenerateRoute(settings); - if (route == null) { - assert(widget.onUnknownRoute != null); + if (route == null && !allowNull) { + assert(() { + if (widget.onUnknownRoute == null) { + throw new FlutterError( + 'If a Navigator has no onUnknownRoute, then its onGenerateRoute must never return null.\n' + 'When trying to build the route "$name", onGenerateRoute returned null, but there was no ' + 'onUnknownRoute callback specified.\n' + 'The Navigator was:\n' + ' $this' + ); + } + return true; + }); route = widget.onUnknownRoute(settings); - assert(route != null); + assert(() { + if (route == null) { + throw new FlutterError( + 'A Navigator\'s onUnknownRoute returned null.\n' + 'When trying to build the route "$name", both onGenerateRoute and onUnknownRoute returned ' + 'null. The onUnknownRoute callback should never return null.\n' + 'The Navigator was:\n' + ' $this' + ); + } + return true; + }); } return route; } @@ -1245,7 +1347,6 @@ class NavigatorState extends State with TickerProviderStateMixin { Widget build(BuildContext context) { assert(!_debugLocked); assert(_history.isNotEmpty); - final Route initialRoute = _history.first; return new Listener( onPointerDown: _handlePointerDown, onPointerUp: _handlePointerUpOrCancel, @@ -1257,7 +1358,7 @@ class NavigatorState extends State with TickerProviderStateMixin { autofocus: true, child: new Overlay( key: _overlayKey, - initialEntries: initialRoute.overlayEntries, + initialEntries: _initialOverlayEntries, ), ), ), diff --git a/packages/flutter/test/material/app_test.dart b/packages/flutter/test/material/app_test.dart index 92249a5ae1..470e4973c8 100644 --- a/packages/flutter/test/material/app_test.dart +++ b/packages/flutter/test/material/app_test.dart @@ -152,34 +152,65 @@ void main() { expect(find.text('route "/"'), findsOneWidget); }); - testWidgets('Custom initialRoute only', (WidgetTester tester) async { + testWidgets('One-step initial route', (WidgetTester tester) async { await tester.pumpWidget( new MaterialApp( initialRoute: '/a', routes: { + '/': (BuildContext context) => const Text('route "/"'), '/a': (BuildContext context) => const Text('route "/a"'), + '/a/b': (BuildContext context) => const Text('route "/a/b"'), + '/b': (BuildContext context) => const Text('route "/b"'), }, ) ); + expect(find.text('route "/"'), findsOneWidget); expect(find.text('route "/a"'), findsOneWidget); + expect(find.text('route "/a/b"'), findsNothing); + expect(find.text('route "/b"'), findsNothing); }); - testWidgets('Custom initialRoute along with Navigator.defaultRouteName', (WidgetTester tester) async { + testWidgets('Two-step initial route', (WidgetTester tester) async { final Map routes = { '/': (BuildContext context) => const Text('route "/"'), '/a': (BuildContext context) => const Text('route "/a"'), + '/a/b': (BuildContext context) => const Text('route "/a/b"'), '/b': (BuildContext context) => const Text('route "/b"'), }; await tester.pumpWidget( new MaterialApp( - initialRoute: '/a', + initialRoute: '/a/b', routes: routes, ) ); - expect(find.text('route "/"'), findsNothing); + expect(find.text('route "/"'), findsOneWidget); expect(find.text('route "/a"'), findsOneWidget); + expect(find.text('route "/a/b"'), findsOneWidget); + expect(find.text('route "/b"'), findsNothing); + }); + + testWidgets('Initial route with missing step', (WidgetTester tester) async { + final Map routes = { + '/': (BuildContext context) => const Text('route "/"'), + '/a': (BuildContext context) => const Text('route "/a"'), + '/a/b': (BuildContext context) => const Text('route "/a/b"'), + '/b': (BuildContext context) => const Text('route "/b"'), + }; + + await tester.pumpWidget( + new MaterialApp( + initialRoute: '/a/b/c', + routes: routes, + ) + ); + final dynamic exception = tester.takeException(); + expect(exception is String, isTrue); + expect(exception.startsWith('Could not navigate to initial route.'), isTrue); + expect(find.text('route "/"'), findsOneWidget); + expect(find.text('route "/a"'), findsNothing); + expect(find.text('route "/a/b"'), findsNothing); expect(find.text('route "/b"'), findsNothing); }); @@ -196,23 +227,41 @@ void main() { routes: routes, ) ); - expect(find.text('route "/"'), findsNothing); + expect(find.text('route "/"'), findsOneWidget); expect(find.text('route "/a"'), findsOneWidget); expect(find.text('route "/b"'), findsNothing); + // changing initialRoute has no effect await tester.pumpWidget( new MaterialApp( initialRoute: '/b', routes: routes, ) ); - expect(find.text('route "/"'), findsNothing); + expect(find.text('route "/"'), findsOneWidget); expect(find.text('route "/a"'), findsOneWidget); expect(find.text('route "/b"'), findsNothing); + // removing it has no effect await tester.pumpWidget(new MaterialApp(routes: routes)); - expect(find.text('route "/"'), findsNothing); + expect(find.text('route "/"'), findsOneWidget); expect(find.text('route "/a"'), findsOneWidget); expect(find.text('route "/b"'), findsNothing); }); + + testWidgets('onGenerateRoute / onUnknownRoute', (WidgetTester tester) async { + final List log = []; + await tester.pumpWidget( + new MaterialApp( + onGenerateRoute: (RouteSettings settings) { + log.add('onGenerateRoute ${settings.name}'); + }, + onUnknownRoute: (RouteSettings settings) { + log.add('onUnknownRoute ${settings.name}'); + }, + ) + ); + expect(tester.takeException(), isFlutterError); + expect(log, ['onGenerateRoute /', 'onUnknownRoute /']); + }); } diff --git a/packages/flutter/test/widgets/routes_test.dart b/packages/flutter/test/widgets/routes_test.dart index bda498bd34..4ff567d076 100644 --- a/packages/flutter/test/widgets/routes_test.dart +++ b/packages/flutter/test/widgets/routes_test.dart @@ -98,6 +98,12 @@ void main() { testWidgets('Route settings', (WidgetTester tester) async { final RouteSettings settings = const RouteSettings(name: 'A'); expect(settings, hasOneLineDescription); + final RouteSettings settings2 = settings.copyWith(name: 'B'); + expect(settings2.name, 'B'); + expect(settings2.isInitialRoute, false); + final RouteSettings settings3 = settings2.copyWith(isInitialRoute: true); + expect(settings3.name, 'B'); + expect(settings3.isInitialRoute, true); }); testWidgets('Route management - push, replace, pop', (WidgetTester tester) async { diff --git a/packages/flutter_driver/lib/driver_extension.dart b/packages/flutter_driver/lib/driver_extension.dart index 15d3eae3f9..caac00edad 100644 --- a/packages/flutter_driver/lib/driver_extension.dart +++ b/packages/flutter_driver/lib/driver_extension.dart @@ -24,4 +24,4 @@ /// } library flutter_driver_extension; -export 'src/extension.dart' show enableFlutterDriverExtension; +export 'src/extension.dart' show enableFlutterDriverExtension, DataHandler; diff --git a/packages/flutter_driver/lib/src/driver.dart b/packages/flutter_driver/lib/src/driver.dart index edd18518bf..c3ed994873 100644 --- a/packages/flutter_driver/lib/src/driver.dart +++ b/packages/flutter_driver/lib/src/driver.dart @@ -21,6 +21,7 @@ import 'gesture.dart'; import 'health.dart'; import 'message.dart'; import 'render_tree.dart'; +import 'request_data.dart'; import 'semantics.dart'; import 'timeline.dart'; @@ -384,6 +385,13 @@ class FlutterDriver { return GetTextResult.fromJson(await _sendCommand(new GetText(finder, timeout: timeout))).text; } + /// Sends a string and returns a string. + /// + /// The application can respond to this by providing a handler to [enableFlutterDriverExtension]. + Future requestData(String message, { Duration timeout }) async { + return RequestDataResult.fromJson(await _sendCommand(new RequestData(message, timeout: timeout))).message; + } + /// Turns semantics on or off in the Flutter app under test. /// /// Returns `true` when the call actually changed the state from on to off or diff --git a/packages/flutter_driver/lib/src/extension.dart b/packages/flutter_driver/lib/src/extension.dart index cb5ff5fcc9..6a617cc57b 100644 --- a/packages/flutter_driver/lib/src/extension.dart +++ b/packages/flutter_driver/lib/src/extension.dart @@ -6,10 +6,12 @@ import 'dart:async'; import 'package:meta/meta.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart' show RendererBinding, SemanticsHandle; import 'package:flutter/scheduler.dart'; +import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -20,19 +22,26 @@ import 'gesture.dart'; import 'health.dart'; import 'message.dart'; import 'render_tree.dart'; +import 'request_data.dart'; import 'semantics.dart'; const String _extensionMethodName = 'driver'; const String _extensionMethod = 'ext.flutter.$_extensionMethodName'; -class _DriverBinding extends WidgetsFlutterBinding { // TODO(ianh): refactor so we're not extending a concrete binding +typedef Future DataHandler(String message); + +class _DriverBinding extends BindingBase with SchedulerBinding, GestureBinding, ServicesBinding, RendererBinding, WidgetsBinding { + _DriverBinding(this._handler); + + final DataHandler _handler; + @override void initServiceExtensions() { super.initServiceExtensions(); - final FlutterDriverExtension extension = new FlutterDriverExtension(); + final FlutterDriverExtension extension = new FlutterDriverExtension(_handler); registerServiceExtension( name: _extensionMethodName, - callback: extension.call + callback: extension.call, ); } } @@ -44,9 +53,12 @@ class _DriverBinding extends WidgetsFlutterBinding { // TODO(ianh): refactor so /// /// Call this function prior to running your application, e.g. before you call /// `runApp`. -void enableFlutterDriverExtension() { +/// +/// Optionally you can pass a [DataHandler] callback. It will be called if the +/// test calls [FlutterDriver.requestData]. +void enableFlutterDriverExtension({ DataHandler handler }) { assert(WidgetsBinding.instance == null); - new _DriverBinding(); + new _DriverBinding(handler); assert(WidgetsBinding.instance is _DriverBinding); } @@ -62,18 +74,17 @@ typedef Finder FinderConstructor(SerializableFinder finder); @visibleForTesting class FlutterDriverExtension { - static final Logger _log = new Logger('FlutterDriverExtension'); - - FlutterDriverExtension() { + FlutterDriverExtension(this._requestDataHandler) { _commandHandlers.addAll({ 'get_health': _getHealth, 'get_render_tree': _getRenderTree, - 'tap': _tap, 'get_text': _getText, - 'set_frame_sync': _setFrameSync, - 'set_semantics': _setSemantics, + 'request_data': _requestData, 'scroll': _scroll, 'scrollIntoView': _scrollIntoView, + 'set_frame_sync': _setFrameSync, + 'set_semantics': _setSemantics, + 'tap': _tap, 'waitFor': _waitFor, 'waitUntilNoTransientCallbacks': _waitUntilNoTransientCallbacks, }); @@ -81,12 +92,13 @@ class FlutterDriverExtension { _commandDeserializers.addAll({ 'get_health': (Map params) => new GetHealth.deserialize(params), 'get_render_tree': (Map params) => new GetRenderTree.deserialize(params), - 'tap': (Map params) => new Tap.deserialize(params), 'get_text': (Map params) => new GetText.deserialize(params), - 'set_frame_sync': (Map params) => new SetFrameSync.deserialize(params), - 'set_semantics': (Map params) => new SetSemantics.deserialize(params), + 'request_data': (Map params) => new RequestData.deserialize(params), 'scroll': (Map params) => new Scroll.deserialize(params), 'scrollIntoView': (Map params) => new ScrollIntoView.deserialize(params), + 'set_frame_sync': (Map params) => new SetFrameSync.deserialize(params), + 'set_semantics': (Map params) => new SetSemantics.deserialize(params), + 'tap': (Map params) => new Tap.deserialize(params), 'waitFor': (Map params) => new WaitFor.deserialize(params), 'waitUntilNoTransientCallbacks': (Map params) => new WaitUntilNoTransientCallbacks.deserialize(params), }); @@ -98,6 +110,10 @@ class FlutterDriverExtension { }); } + final DataHandler _requestDataHandler; + + static final Logger _log = new Logger('FlutterDriverExtension'); + final WidgetController _prober = new WidgetController(WidgetsBinding.instance); final Map _commandHandlers = {}; final Map _commandDeserializers = {}; @@ -117,6 +133,7 @@ class FlutterDriverExtension { /// /// The returned JSON is command specific. Generally the caller deserializes /// the result into a subclass of [Result], but that's not strictly required. + @visibleForTesting Future> call(Map params) async { final String commandKind = params['command']; try { @@ -243,8 +260,8 @@ class FlutterDriverExtension { _prober.binding.hitTest(hitTest, startLocation); _prober.binding.dispatchEvent(pointer.down(startLocation), hitTest); - await new Future.value(); // so that down and move don't happen in the same microtask - for (int moves = 0; moves < totalMoves; moves++) { + await new Future.value(); // so that down and move don't happen in the same microtask + for (int moves = 0; moves < totalMoves; moves += 1) { currentLocation = currentLocation + delta; _prober.binding.dispatchEvent(pointer.move(currentLocation), hitTest); await new Future.delayed(pause); @@ -269,6 +286,11 @@ class FlutterDriverExtension { return new GetTextResult(text.data); } + Future _requestData(Command command) async { + final RequestData requestDataCommand = command; + return new RequestDataResult(_requestDataHandler == null ? '' : await _requestDataHandler(requestDataCommand.message)); + } + Future _setFrameSync(Command command) async { final SetFrameSync setFrameSyncCommand = command; _frameSync = setFrameSyncCommand.enabled; diff --git a/packages/flutter_driver/lib/src/frame_sync.dart b/packages/flutter_driver/lib/src/frame_sync.dart index ad008ef269..dea7bc5549 100644 --- a/packages/flutter_driver/lib/src/frame_sync.dart +++ b/packages/flutter_driver/lib/src/frame_sync.dart @@ -9,11 +9,11 @@ class SetFrameSync extends Command { @override final String kind = 'set_frame_sync'; + SetFrameSync(this.enabled, { Duration timeout }) : super(timeout: timeout); + /// Whether frameSync should be enabled or disabled. final bool enabled; - SetFrameSync(this.enabled, { Duration timeout }) : super(timeout: timeout); - /// Deserializes this command from the value generated by [serialize]. SetFrameSync.deserialize(Map params) : this.enabled = params['enabled'].toLowerCase() == 'true', diff --git a/packages/flutter_driver/lib/src/health.dart b/packages/flutter_driver/lib/src/health.dart index da10a27e89..dc29205930 100644 --- a/packages/flutter_driver/lib/src/health.dart +++ b/packages/flutter_driver/lib/src/health.dart @@ -10,12 +10,14 @@ class GetHealth extends Command { @override final String kind = 'get_health'; + /// Create a health check command. GetHealth({Duration timeout}) : super(timeout: timeout); /// Deserializes the command from JSON generated by [serialize]. GetHealth.deserialize(Map json) : super.deserialize(json); } +/// A description of application state. enum HealthStatus { /// Application is known to be in a good shape and should be able to respond. ok, @@ -27,6 +29,8 @@ enum HealthStatus { final EnumIndex _healthStatusIndex = new EnumIndex(HealthStatus.values); +/// A description of the application state, as provided in response to a +/// [FlutterDriver.checkHealth] test. class Health extends Result { /// Creates a [Health] object with the given [status]. Health(this.status) { @@ -38,10 +42,13 @@ class Health extends Result { return new Health(_healthStatusIndex.lookupBySimpleName(json['status'])); } + /// The status represented by this object. + /// + /// If the application responded, this will be [HealthStatus.ok]. final HealthStatus status; @override Map toJson() => { - 'status': _healthStatusIndex.toSimpleName(status) + 'status': _healthStatusIndex.toSimpleName(status), }; } diff --git a/packages/flutter_driver/lib/src/request_data.dart b/packages/flutter_driver/lib/src/request_data.dart new file mode 100644 index 0000000000..097e06727d --- /dev/null +++ b/packages/flutter_driver/lib/src/request_data.dart @@ -0,0 +1,46 @@ +// Copyright 2016 The Chromium 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 'message.dart'; + +/// Send a string and get a string response. +class RequestData extends Command { + @override + final String kind = 'request_data'; + + /// Create a command that sends a message. + RequestData(this.message, { Duration timeout }) : super(timeout: timeout); + + /// The message being sent from the test to the application. + final String message; + + /// Deserializes this command from the value generated by [serialize]. + RequestData.deserialize(Map params) + : this.message = params['message'], + super.deserialize(params); + + @override + Map serialize() => super.serialize()..addAll({ + 'message': message, + }); +} + +/// The result of the [RequestData] command. +class RequestDataResult extends Result { + /// Creates a result with the given [message]. + RequestDataResult(this.message); + + /// The text extracted by the [RequestData] command. + final String message; + + /// Deserializes the result from JSON. + static RequestDataResult fromJson(Map json) { + return new RequestDataResult(json['message']); + } + + @override + Map toJson() => { + 'message': message, + }; +} diff --git a/packages/flutter_driver/lib/src/semantics.dart b/packages/flutter_driver/lib/src/semantics.dart index b9af7c15be..83cfe8d298 100644 --- a/packages/flutter_driver/lib/src/semantics.dart +++ b/packages/flutter_driver/lib/src/semantics.dart @@ -9,11 +9,11 @@ class SetSemantics extends Command { @override final String kind = 'set_semantics'; + SetSemantics(this.enabled, { Duration timeout }) : super(timeout: timeout); + /// Whether semantics should be enabled or disabled. final bool enabled; - SetSemantics(this.enabled, { Duration timeout }) : super(timeout: timeout); - /// Deserializes this command from the value generated by [serialize]. SetSemantics.deserialize(Map params) : this.enabled = params['enabled'].toLowerCase() == 'true', diff --git a/packages/flutter_driver/test/src/extension_test.dart b/packages/flutter_driver/test/src/extension_test.dart index f341033677..0aec947156 100644 --- a/packages/flutter_driver/test/src/extension_test.dart +++ b/packages/flutter_driver/test/src/extension_test.dart @@ -5,16 +5,19 @@ import 'package:flutter/scheduler.dart'; import 'package:flutter_driver/src/extension.dart'; import 'package:flutter_driver/src/find.dart'; +import 'package:flutter_driver/src/request_data.dart'; import 'package:flutter_test/flutter_test.dart'; void main() { group('waitUntilNoTransientCallbacks', () { FlutterDriverExtension extension; Map result; + int messageId = 0; + final List log = []; setUp(() { result = null; - extension = new FlutterDriverExtension(); + extension = new FlutterDriverExtension((String message) async { log.add(message); return (messageId += 1).toString(); }); }); testWidgets('returns immediately when transient callback queue is empty', (WidgetTester tester) async { @@ -57,5 +60,12 @@ void main() { }, ); }); + + testWidgets('handler', (WidgetTester tester) async { + expect(log, isEmpty); + final dynamic result = RequestDataResult.fromJson((await extension.call(new RequestData('hello').serialize()))['response']); + expect(log, ['hello']); + expect(result.message, '1'); + }); }); } diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index 0bb14a0e74..c5e0a97903 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/packages/flutter_tools/lib/src/commands/drive.dart @@ -42,19 +42,22 @@ class DriveCommand extends RunCommandBase { argParser.addFlag( 'keep-app-running', negatable: true, - defaultsTo: false, help: 'Will keep the Flutter application running when done testing.\n' - 'By default, Flutter drive stops the application after tests are finished.\n' - 'Ignored if --use-existing-app is specified.' + 'By default, "flutter drive" stops the application after tests are finished,\n' + 'and --keep-app-running overrides this. On the other hand, if --use-existing-app\n' + 'is specified, then "flutter drive" instead defaults to leaving the application\n' + 'running, and --no-keep-app-running overrides it.' ); argParser.addOption( 'use-existing-app', help: 'Connect to an already running instance via the given observatory URL.\n' - 'If this option is given, the application will not be automatically started\n' - 'or stopped.' + 'If this option is given, the application will not be automatically started,\n' + 'and it will only be stopped if --no-keep-app-running is explicitly set.', + valueHelp: + 'url' ); } @@ -95,7 +98,7 @@ class DriveCommand extends RunCommandBase { String observatoryUri; if (argResults['use-existing-app'] == null) { - printStatus('Starting application: ${argResults["target"]}'); + printStatus('Starting application: $targetFile'); if (getBuildMode() == BuildMode.release) { // This is because we need VM service to be able to drive the app. @@ -125,11 +128,11 @@ class DriveCommand extends RunCommandBase { rethrow; throwToolExit('CAUGHT EXCEPTION: $error\n$stackTrace'); } finally { - if (!argResults['keep-app-running'] && argResults['use-existing-app'] == null) { + if (argResults['keep-app-running'] ?? (argResults['use-existing-app'] != null)) { + printStatus('Leaving the application running.'); + } else { printStatus('Stopping application instance.'); await appStopper(this); - } else { - printStatus('Leaving the application running.'); } } } @@ -137,7 +140,7 @@ class DriveCommand extends RunCommandBase { String _getTestFile() { String appFile = fs.path.normalize(targetFile); - // This command extends `flutter start` and therefore CWD == package dir + // This command extends `flutter run` and therefore CWD == package dir final String packageDir = fs.currentDirectory.path; // Make appFile path relative to package directory because we are looking @@ -209,7 +212,7 @@ Future findTargetDevice() async { /// Starts the application on the device given command configuration. typedef Future AppStarter(DriveCommand command); -AppStarter appStarter = _startApp; +AppStarter appStarter = _startApp; // (mutable for testing) void restoreAppStarter() { appStarter = _startApp; } @@ -255,7 +258,7 @@ Future _startApp(DriveCommand command) async { observatoryPort: command.observatoryPort, diagnosticPort: command.diagnosticPort, ), - platformArgs: platformArgs + platformArgs: platformArgs, ); if (!result.started) { diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index bc596fb1b0..397a93b6d5 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -317,8 +317,8 @@ class RunCommand extends RunCommandBase { } DateTime appStartedTime; - // Sync completer so the completing agent attaching to the resident doesn't - // need to know about analytics. + // Sync completer so the completing agent attaching to the resident doesn't + // need to know about analytics. // // Do not add more operations to the future. final Completer appStartedTimeRecorder = new Completer.sync(); @@ -338,7 +338,7 @@ class RunCommand extends RunCommandBase { analyticsParameters: [ hotMode ? 'hot' : 'cold', getModeName(getBuildMode()), - devices.length == 1 + devices.length == 1 ? getNameForTargetPlatform(await devices[0].targetPlatform) : 'multiple', devices.length == 1 && await devices[0].isLocalEmulator ? 'emulator' : null diff --git a/packages/flutter_tools/lib/src/test/watcher.dart b/packages/flutter_tools/lib/src/test/watcher.dart index 114f456151..497531592a 100644 --- a/packages/flutter_tools/lib/src/test/watcher.dart +++ b/packages/flutter_tools/lib/src/test/watcher.dart @@ -7,7 +7,6 @@ import '../base/io.dart' show Process; /// Callbacks for reporting progress while running tests. class TestWatcher { - /// Called after a child process starts. /// /// If startPaused was true, the caller needs to resume in Observatory to