diff --git a/.cirrus.yml b/.cirrus.yml index 286e8a6dd1..1d37d5fa9d 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -4,9 +4,9 @@ web_shard_template: &WEB_SHARD_TEMPLATE only_if: "changesInclude('.cirrus.yml', 'dev/**', 'packages/flutter/**', 'packages/flutter_test/**', 'packages/flutter_tools/lib/src/test/**', 'packages/flutter_web_plugins/**', 'bin/internal/**') || $CIRRUS_PR == ''" environment: - # As of October 2019, the Web shards needed more than 6G of RAM. - CPU: 2 - MEMORY: 8G + # As of March 2020, the Web shards needed 16G of RAM and 4 CPUs to run all framework tests with goldens without flaking. + CPU: 4 + MEMORY: 16G CHROME_NO_SANDBOX: true GOLD_SERVICE_ACCOUNT: ENCRYPTED[3afeea5ac7201151c3d0dc9648862f0462b5e4f55dc600ca8b692319622f7c3eda3d577b1b16cc2ef0311b7314c1c095] script: diff --git a/dev/bots/test.dart b/dev/bots/test.dart index 32efaec3da..37ff25b3c9 100644 --- a/dev/bots/test.dart +++ b/dev/bots/test.dart @@ -53,11 +53,6 @@ const int kDeviceLabShardCount = 4; /// The last shard also runs the Web plugin tests. const int kWebShardCount = 8; -/// Maximum number of Web tests to run in a single `flutter test`. We found that -/// large batches can get flaky, possibly because we reuse a single instance of -/// the browser, and after many tests the browser's state gets corrupted. -const int kWebBatchSize = 20; - /// Tests that we don't run on Web for various reasons. // // TODO(yjbanov): we're getting rid of this blacklist as part of https://github.com/flutter/flutter/projects/60 @@ -685,31 +680,23 @@ Future _runWebDebugTest(String target) async { } Future _runFlutterWebTest(String workingDirectory, List tests) async { - final List batch = []; - for (int i = 0; i < tests.length; i += 1) { - final String testFilePath = tests[i]; - batch.add(testFilePath); - if (batch.length == kWebBatchSize || i == tests.length - 1) { - await runCommand( - flutter, - [ - 'test', - if (ciProvider == CiProviders.cirrus) - '--concurrency=1', // do not parallelize on Cirrus, to reduce flakiness - '-v', - '--platform=chrome', - ...?flutterTestArgs, - ...batch, - ], - workingDirectory: workingDirectory, - environment: { - 'FLUTTER_WEB': 'true', - 'FLUTTER_LOW_RESOURCE_MODE': 'true', - }, - ); - batch.clear(); - } - } + await runCommand( + flutter, + [ + 'test', + if (ciProvider == CiProviders.cirrus) + '--concurrency=1', // do not parallelize on Cirrus, to reduce flakiness + '-v', + '--platform=chrome', + ...?flutterTestArgs, + ...tests, + ], + workingDirectory: workingDirectory, + environment: { + 'FLUTTER_WEB': 'true', + 'FLUTTER_LOW_RESOURCE_MODE': 'true', + }, + ); } Future _pubRunTest(String workingDirectory, { diff --git a/packages/flutter/test/material/bottom_app_bar_theme_test.dart b/packages/flutter/test/material/bottom_app_bar_theme_test.dart index aad4876d45..d3aa2224e7 100644 --- a/packages/flutter/test/material/bottom_app_bar_theme_test.dart +++ b/packages/flutter/test/material/bottom_app_bar_theme_test.dart @@ -82,7 +82,7 @@ void main() { find.byKey(_painterKey), matchesGoldenFile('bottom_app_bar_theme.custom_shape.png'), ); - }, skip: isBrowser); + }); testWidgets('BAB theme does not affect defaults', (WidgetTester tester) async { await tester.pumpWidget(const MaterialApp( diff --git a/packages/flutter/test/material/bottom_navigation_bar_test.dart b/packages/flutter/test/material/bottom_navigation_bar_test.dart index f17de80fef..793611eea7 100644 --- a/packages/flutter/test/material/bottom_navigation_bar_test.dart +++ b/packages/flutter/test/material/bottom_navigation_bar_test.dart @@ -913,7 +913,7 @@ void main() { await tester.tap(find.text('Alarm')); await tester.pump(const Duration(seconds: 1)); expect(Theme.of(tester.element(find.text('Alarm'))).brightness, equals(Brightness.dark)); - }, skip: isBrowser); + }); testWidgets('BottomNavigationBar iconSize test', (WidgetTester tester) async { double builderIconSize; @@ -1023,7 +1023,7 @@ void main() { final RenderBox box = tester.renderObject(find.byType(BottomNavigationBar)); expect(box.size.height, equals(66.0)); - }, skip: isBrowser); + }); testWidgets('BottomNavigationBar limits width of tiles with long titles', (WidgetTester tester) async { final Text longTextA = Text(''.padLeft(100, 'A')); @@ -1055,7 +1055,7 @@ void main() { expect(itemBoxA.size, equals(const Size(400.0, 14.0))); final RenderBox itemBoxB = tester.renderObject(find.text(longTextB.data)); expect(itemBoxB.size, equals(const Size(400.0, 14.0))); - }, skip: isBrowser); + }); testWidgets('BottomNavigationBar paints circles', (WidgetTester tester) async { await tester.pumpWidget( @@ -1125,7 +1125,7 @@ void main() { ..translate(x: 400.0) ..circle(x: 200.0), ); - }, skip: isBrowser); + }); testWidgets('BottomNavigationBar inactiveIcon shown', (WidgetTester tester) async { const Key filled = Key('filled'); @@ -1452,7 +1452,7 @@ void main() { find.byType(BottomNavigationBar), matchesGoldenFile('bottom_navigation_bar.shifting_transition.${pump - 1}.png'), ); - }, skip: isBrowser); // TODO(yjbanov): web does not support golden tests yet: https://github.com/flutter/flutter/issues/40297 + }); } }); diff --git a/packages/flutter/test/material/dialog_theme_test.dart b/packages/flutter/test/material/dialog_theme_test.dart index fbbc3a9166..a720435669 100644 --- a/packages/flutter/test/material/dialog_theme_test.dart +++ b/packages/flutter/test/material/dialog_theme_test.dart @@ -132,7 +132,7 @@ void main() { find.byKey(_painterKey), matchesGoldenFile('dialog_theme.dialog_with_custom_border.png'), ); - }, skip: isBrowser); + }); testWidgets('Custom Title Text Style - Constructor Param', (WidgetTester tester) async { const String titleText = 'Title'; diff --git a/packages/flutter/test/material/dropdown_test.dart b/packages/flutter/test/material/dropdown_test.dart index 5b2d265fe4..08ddf9fd8e 100644 --- a/packages/flutter/test/material/dropdown_test.dart +++ b/packages/flutter/test/material/dropdown_test.dart @@ -189,7 +189,7 @@ void main() { find.ancestor(of: buttonFinder, matching: find.byType(RepaintBoundary)).first, matchesGoldenFile('dropdown_test.default.png'), ); - }, skip: isBrowser); + }); testWidgets('Expanded dropdown golden', (WidgetTester tester) async { final Key buttonKey = UniqueKey(); @@ -201,7 +201,7 @@ void main() { find.ancestor(of: buttonFinder, matching: find.byType(RepaintBoundary)).first, matchesGoldenFile('dropdown_test.expanded.png'), ); - }, skip: isBrowser); + }); testWidgets('Dropdown button control test', (WidgetTester tester) async { String value = 'one'; diff --git a/packages/flutter/test/material/flexible_space_bar_stretch_mode_test.dart b/packages/flutter/test/material/flexible_space_bar_stretch_mode_test.dart index 8188d6ed54..09077feb0c 100644 --- a/packages/flutter/test/material/flexible_space_bar_stretch_mode_test.dart +++ b/packages/flutter/test/material/flexible_space_bar_stretch_mode_test.dart @@ -85,7 +85,7 @@ void main() { find.byType(FlexibleSpaceBar), matchesGoldenFile('flexible_space_bar_stretch_mode.blur_background.png'), ); - }, skip: isBrowser); + }); testWidgets('FlexibleSpaceBar stretch mode fadeTitle', (WidgetTester tester) async { await tester.pumpWidget( diff --git a/packages/flutter/test/material/radio_test.dart b/packages/flutter/test/material/radio_test.dart index 472403abe3..272512c088 100644 --- a/packages/flutter/test/material/radio_test.dart +++ b/packages/flutter/test/material/radio_test.dart @@ -282,7 +282,7 @@ void main() { find.byKey(painterKey), matchesGoldenFile('radio.ink_ripple.png'), ); - }, skip: isBrowser); + }); testWidgets('Radio is focusable and has correct focus color', (WidgetTester tester) async { final FocusNode focusNode = FocusNode(debugLabel: 'Radio'); diff --git a/packages/flutter/test/material/tab_bar_theme_test.dart b/packages/flutter/test/material/tab_bar_theme_test.dart index c2aebf034b..cbdd97ae7a 100644 --- a/packages/flutter/test/material/tab_bar_theme_test.dart +++ b/packages/flutter/test/material/tab_bar_theme_test.dart @@ -269,7 +269,7 @@ void main() { find.byKey(_painterKey), matchesGoldenFile('tab_bar_theme.tab_indicator_size_tab.png'), ); - }, skip: isBrowser); + }); testWidgets('Tab bar theme overrides tab indicator size (label)', (WidgetTester tester) async { const TabBarTheme tabBarTheme = TabBarTheme(indicatorSize: TabBarIndicatorSize.label); @@ -280,7 +280,7 @@ void main() { find.byKey(_painterKey), matchesGoldenFile('tab_bar_theme.tab_indicator_size_label.png'), ); - }, skip: isBrowser); + }); testWidgets('Tab bar theme - custom tab indicator', (WidgetTester tester) async { final TabBarTheme tabBarTheme = TabBarTheme( @@ -296,7 +296,7 @@ void main() { find.byKey(_painterKey), matchesGoldenFile('tab_bar_theme.custom_tab_indicator.png'), ); - }, skip: isBrowser); + }); testWidgets('Tab bar theme - beveled rect indicator', (WidgetTester tester) async { final TabBarTheme tabBarTheme = TabBarTheme( @@ -312,5 +312,5 @@ void main() { find.byKey(_painterKey), matchesGoldenFile('tab_bar_theme.beveled_rect_indicator.png'), ); - }, skip: isBrowser); + }); } diff --git a/packages/flutter/test/rendering/localized_fonts_test.dart b/packages/flutter/test/rendering/localized_fonts_test.dart index 0c3995ae8f..f3eb9f7242 100644 --- a/packages/flutter/test/rendering/localized_fonts_test.dart +++ b/packages/flutter/test/rendering/localized_fonts_test.dart @@ -52,7 +52,6 @@ void main() { matchesGoldenFile('localized_fonts.rich_text.styled_text_span.png'), ); }, - skip: isBrowser, // TODO(yjbanov): implement goldens on the Web: https://github.com/flutter/flutter/issues/40297 ); testWidgets( diff --git a/packages/flutter/test/widgets/backdrop_filter_test.dart b/packages/flutter/test/widgets/backdrop_filter_test.dart index 6bfcbd9229..6d25448b23 100644 --- a/packages/flutter/test/widgets/backdrop_filter_test.dart +++ b/packages/flutter/test/widgets/backdrop_filter_test.dart @@ -45,5 +45,5 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('backdrop_filter_test.cull_rect.png'), ); - }, skip: isBrowser); + }); } diff --git a/packages/flutter/test/widgets/clip_test.dart b/packages/flutter/test/widgets/clip_test.dart index 94f1924844..2136bb44af 100644 --- a/packages/flutter/test/widgets/clip_test.dart +++ b/packages/flutter/test/widgets/clip_test.dart @@ -383,7 +383,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.ClipRect.png'), ); - }, skip: isBrowser); + }); testWidgets('ClipRect save, overlay, and antialiasing', (WidgetTester tester) async { await tester.pumpWidget( @@ -423,7 +423,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.ClipRectOverlay.png'), ); - }, skip: isBrowser); + }); testWidgets('ClipRRect painting', (WidgetTester tester) async { await tester.pumpWidget( @@ -472,7 +472,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.ClipRRect.png'), ); - }, skip: isBrowser); + }); testWidgets('ClipOval painting', (WidgetTester tester) async { await tester.pumpWidget( @@ -515,7 +515,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.ClipOval.png'), ); - }, skip: isBrowser); + }); testWidgets('ClipPath painting', (WidgetTester tester) async { await tester.pumpWidget( @@ -563,7 +563,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.ClipPath.png'), ); - }, skip: isBrowser); + }); Center genPhysicalModel(Clip clipBehavior) { return Center( @@ -608,7 +608,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.PhysicalModel.antiAlias.png'), ); - }, skip: isBrowser); + }); testWidgets('PhysicalModel painting with Clip.hardEdge', (WidgetTester tester) async { await tester.pumpWidget(genPhysicalModel(Clip.hardEdge)); @@ -616,7 +616,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.PhysicalModel.hardEdge.png'), ); - }, skip: isBrowser); + }); // There will be bleeding edges on the rect edges, but there shouldn't be any bleeding edges on the // round corners. @@ -626,7 +626,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.PhysicalModel.antiAliasWithSaveLayer.png'), ); - }, skip: isBrowser); + }); testWidgets('Default PhysicalModel painting', (WidgetTester tester) async { await tester.pumpWidget( @@ -668,7 +668,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.PhysicalModel.default.png'), ); - }, skip: isBrowser); + }); Center genPhysicalShape(Clip clipBehavior) { return Center( @@ -717,7 +717,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.PhysicalShape.antiAlias.png'), ); - }, skip: isBrowser); + }); testWidgets('PhysicalShape painting with Clip.hardEdge', (WidgetTester tester) async { await tester.pumpWidget(genPhysicalShape(Clip.hardEdge)); @@ -725,7 +725,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.PhysicalShape.hardEdge.png'), ); - }, skip: isBrowser); + }); testWidgets('PhysicalShape painting with Clip.antiAliasWithSaveLayer', (WidgetTester tester) async { await tester.pumpWidget(genPhysicalShape(Clip.antiAliasWithSaveLayer)); @@ -733,7 +733,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.PhysicalShape.antiAliasWithSaveLayer.png'), ); - }, skip: isBrowser); + }); testWidgets('PhysicalShape painting', (WidgetTester tester) async { await tester.pumpWidget( @@ -779,7 +779,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('clip.PhysicalShape.default.png'), ); - }, skip: isBrowser); + }); testWidgets('ClipPath.shape', (WidgetTester tester) async { final List logs = []; diff --git a/packages/flutter/test/widgets/invert_colors_test.dart b/packages/flutter/test/widgets/invert_colors_test.dart index fc4164897d..f899d60ecc 100644 --- a/packages/flutter/test/widgets/invert_colors_test.dart +++ b/packages/flutter/test/widgets/invert_colors_test.dart @@ -22,7 +22,7 @@ void main() { find.byType(RepaintBoundary), matchesGoldenFile('invert_colors_test.0.png'), ); - }, skip: isBrowser); + }); testWidgets('InvertColors and ColorFilter', (WidgetTester tester) async { await tester.pumpWidget(const RepaintBoundary( @@ -40,7 +40,7 @@ void main() { find.byType(RepaintBoundary), matchesGoldenFile('invert_colors_test.1.png'), ); - }, skip: isBrowser); + }); } // Draws a rectangle sized by the parent widget with [color], [colorFilter], diff --git a/packages/flutter/test/widgets/list_wheel_scroll_view_test.dart b/packages/flutter/test/widgets/list_wheel_scroll_view_test.dart index 0d86ddb963..4de089f13b 100644 --- a/packages/flutter/test/widgets/list_wheel_scroll_view_test.dart +++ b/packages/flutter/test/widgets/list_wheel_scroll_view_test.dart @@ -594,7 +594,7 @@ void main() { find.byKey(const Key('list_wheel_scroll_view')), matchesGoldenFile('list_wheel_scroll_view.center_child.magnified.png'), ); - }, skip: isBrowser); + }); testWidgets('Default middle transform', (WidgetTester tester) async { await tester.pumpWidget( @@ -648,7 +648,7 @@ void main() { find.byKey(const Key('list_wheel_scroll_view')), matchesGoldenFile('list_wheel_scroll_view.curved_wheel.left.png'), ); - }, skip: isBrowser); + }); testWidgets('Scrolling, diameterRatio, perspective all changes matrix', (WidgetTester tester) async { final ScrollController controller = ScrollController(initialScrollOffset: 200.0); diff --git a/packages/flutter/test/widgets/opacity_test.dart b/packages/flutter/test/widgets/opacity_test.dart index bda72f3f15..46e3ca4d7e 100644 --- a/packages/flutter/test/widgets/opacity_test.dart +++ b/packages/flutter/test/widgets/opacity_test.dart @@ -180,7 +180,7 @@ void main() { find.byType(RepaintBoundary).first, matchesGoldenFile('opacity_test.offset.png'), ); - }, skip: isBrowser); + }); testWidgets('empty opacity does not crash', (WidgetTester tester) async { await tester.pumpWidget( diff --git a/packages/flutter/test/widgets/physical_model_test.dart b/packages/flutter/test/widgets/physical_model_test.dart index 0780f5a753..13fc56113a 100644 --- a/packages/flutter/test/widgets/physical_model_test.dart +++ b/packages/flutter/test/widgets/physical_model_test.dart @@ -112,7 +112,7 @@ void main() { find.byKey(key), matchesGoldenFile('physical_model_overflow.png'), ); - }, skip: isBrowser); + }); group('PhysicalModelLayer checks elevation', () { Future _testStackChildren( diff --git a/packages/flutter_tools/lib/src/test/flutter_web_platform.dart b/packages/flutter_tools/lib/src/test/flutter_web_platform.dart index 9434a45432..de39ee8492 100644 --- a/packages/flutter_tools/lib/src/test/flutter_web_platform.dart +++ b/packages/flutter_tools/lib/src/test/flutter_web_platform.dart @@ -256,10 +256,8 @@ class FlutterWebPlatform extends PlatformPlugin { Uint8List bytes; try { - final Runtime browser = Runtime.chrome; - final BrowserManager browserManager = await _browserManagerFor(browser); - final ChromeTab chromeTab = await browserManager._browser.chromeConnection.getTab((ChromeTab tab) { - return tab.url.contains(browserManager._browser.url); + final ChromeTab chromeTab = await _browserManager._browser.chromeConnection.getTab((ChromeTab tab) { + return tab.url.contains(_browserManager._browser.url); }); final WipConnection connection = await chromeTab.connect(); final WipResponse response = await connection.sendCommand('Page.captureScreenshot', { @@ -303,10 +301,7 @@ class FlutterWebPlatform extends PlatformPlugin { bool get _closed => _closeMemo.hasRun; - // A map from browser identifiers to futures that will complete to the - // [BrowserManager]s for those browsers, or `null` if they failed to load. - final Map> _browserManagers = - >{}; + BrowserManager _browserManager; // A handler that serves wrapper files used to bootstrap tests. shelf.Response _wrapperHandler(shelf.Request request) { @@ -330,6 +325,11 @@ class FlutterWebPlatform extends PlatformPlugin { return shelf.Response.notFound('Not found.'); } + /// Allows only one test suite (typically one test file) to be loaded and run + /// at any given point in time. Loading more than one file at a time is known + /// to lead to flaky tests. + final Pool _suiteLock = Pool(1); + @override Future load( String path, @@ -340,17 +340,28 @@ class FlutterWebPlatform extends PlatformPlugin { if (_closed) { return null; } + final PoolResource lockResource = await _suiteLock.request(); + final Runtime browser = platform.runtime; - final BrowserManager browserManager = await _browserManagerFor(browser); - if (_closed || browserManager == null) { + try { + _browserManager = await _launchBrowser(browser); + } on Error catch (_) { + await _suiteLock.close(); + rethrow; + } + + if (_closed) { return null; } final Uri suiteUrl = url.resolveUri(globals.fs.path.toUri(globals.fs.path.withoutExtension( globals.fs.path.relative(path, from: globals.fs.path.join(_root, 'test'))) + '.html')); - final RunnerSuite suite = await browserManager - .load(path, suiteUrl, suiteConfig, message); + final RunnerSuite suite = await _browserManager.load(path, suiteUrl, suiteConfig, message, onDone: () async { + await _browserManager.close(); + _browserManager = null; + lockResource.release(); + }); if (_closed) { return null; } @@ -364,11 +375,11 @@ class FlutterWebPlatform extends PlatformPlugin { /// Returns the [BrowserManager] for [runtime], which should be a browser. /// /// If no browser manager is running yet, starts one. - Future _browserManagerFor(Runtime browser) { - final Future managerFuture = _browserManagers[browser]; - if (managerFuture != null) { - return managerFuture; + Future _launchBrowser(Runtime browser) { + if (_browserManager != null) { + throw StateError('Another browser is currently running.'); } + final Completer completer = Completer.sync(); final String path = @@ -383,48 +394,29 @@ class FlutterWebPlatform extends PlatformPlugin { globals.printTrace('Serving tests at $hostUrl'); - final Future future = BrowserManager.start( + return BrowserManager.start( browser, hostUrl, completer.future, headless: !_config.pauseAfterLoad, ); - - // Store null values for browsers that error out so we know not to load them - // again. - _browserManagers[browser] = future.catchError((dynamic _) => null); - - return future; } @override - Future closeEphemeral() { - final List> managers = - _browserManagers.values.toList(); - _browserManagers.clear(); - return Future.wait(managers.map((Future manager) async { - final BrowserManager result = await manager; - if (result == null) { - return; - } - await result.close(); - })); + Future closeEphemeral() async { + if (_browserManager != null) { + await _browserManager.close(); + } } @override Future close() => _closeMemo.runOnce(() async { - final List> futures = _browserManagers.values - .map>((Future future) async { - final BrowserManager result = await future; - if (result == null) { - return; - } - await result.close(); - }) - .toList(); - futures.add(_server.close()); - futures.add(_testGoldenComparator.close()); - await Future.wait(futures); + await Future.wait(>[ + if (_browserManager != null) + _browserManager.close(), + _server.close(), + _testGoldenComparator.close(), + ]); }); } @@ -578,21 +570,6 @@ class BrowserManager { /// This is connected to a page running `static/host.dart`. MultiChannel _channel; - /// A pool that ensures that limits the number of initial connections the - /// manager will wait for at once. - /// - /// This isn't the *total* number of connections; any number of iframes may be - /// loaded in the same browser. However, the browser can only load so many at - /// once, and we want a timeout in case they fail so we only wait for so many - /// at once. - // The number 1 is chosen to disallow multiple iframes in the same browser. This - // is because in some environments, such as Cirrus CI, tests end up stuck and - // time out eventually. The exact reason for timeouts is unknown, but the - // hypothesis is that we were the first ones to attempt to run DDK-compiled - // tests concurrently in the browser. DDK is known to produce an order of - // magnitude bigger and somewhat slower code, which may overload the browser. - final Pool _pool = Pool(1); - /// The ID of the next suite to be loaded. /// /// This is used to ensure that the suites can be referred to consistently @@ -654,8 +631,8 @@ class BrowserManager { final Completer completer = Completer(); - unawaited(chrome.onExit.then((void _) { - throwToolExit('${runtime.name} exited before connecting.'); + unawaited(chrome.onExit.then((int browserExitCode) { + throwToolExit('${runtime.name} exited with code $browserExitCode before connecting.'); }).catchError((dynamic error, StackTrace stackTrace) { if (completer.isCompleted) { return; @@ -700,7 +677,9 @@ class BrowserManager { String path, Uri url, SuiteConfiguration suiteConfig, - Object message, + Object message, { + Future Function() onDone, + } ) async { url = url.replace(fragment: Uri.encodeFull(jsonEncode({ 'metadata': suiteConfig.metadata.serialize(), @@ -726,29 +705,28 @@ class BrowserManager { StreamTransformer.fromHandlers(handleDone: (EventSink sink) { closeIframe(); sink.close(); + onDone(); }), ); - return await _pool.withResource(() async { - _channel.sink.add({ - 'command': 'loadSuite', - 'url': url.toString(), - 'id': suiteID, - 'channel': suiteChannelID, - }); - - try { - controller = deserializeSuite(path, SuitePlatform(Runtime.chrome), - suiteConfig, await _environment, suiteChannel, message); - - _controllers.add(controller); - return await controller.suite; - // Not limiting to catching Exception because the exception is rethrown. - } catch (_) { // ignore: avoid_catches_without_on_clauses - closeIframe(); - rethrow; - } + _channel.sink.add({ + 'command': 'loadSuite', + 'url': url.toString(), + 'id': suiteID, + 'channel': suiteChannelID, }); + + try { + controller = deserializeSuite(path, SuitePlatform(Runtime.chrome), + suiteConfig, await _environment, suiteChannel, message); + + _controllers.add(controller); + return await controller.suite; + // Not limiting to catching Exception because the exception is rethrown. + } catch (_) { // ignore: avoid_catches_without_on_clauses + closeIframe(); + rethrow; + } } /// An implementation of [Environment.displayPause]. diff --git a/packages/flutter_tools/lib/src/web/chrome.dart b/packages/flutter_tools/lib/src/web/chrome.dart index 55a998bcd4..e28b3be66a 100644 --- a/packages/flutter_tools/lib/src/web/chrome.dart +++ b/packages/flutter_tools/lib/src/web/chrome.dart @@ -116,6 +116,10 @@ class ChromeLauncher { /// /// `skipCheck` does not attempt to make a devtools connection before returning. Future launch(String url, { bool headless = false, int debugPort, bool skipCheck = false, Directory dataDir }) async { + if (_currentCompleter.isCompleted) { + throwToolExit('Only one instance of chrome can be started.'); + } + // This is a JSON file which contains configuration from the // browser session, such as window position. It is located // under the Chrome data-dir folder. @@ -203,9 +207,6 @@ class ChromeLauncher { } static Future _connect(Chrome chrome, bool skipCheck) async { - if (_currentCompleter.isCompleted) { - throwToolExit('Only one instance of chrome can be started.'); - } // The connection is lazy. Try a simple call to make sure the provided // connection is valid. if (!skipCheck) { @@ -262,13 +263,11 @@ class Chrome { final ChromeConnection chromeConnection; final Uri remoteDebuggerUri; - static Completer _currentCompleter = Completer(); - - Future get onExit => _currentCompleter.future; + Future get onExit => _process.exitCode; Future close() async { - if (_currentCompleter.isCompleted) { - _currentCompleter = Completer(); + if (ChromeLauncher.hasChromeInstance) { + ChromeLauncher._currentCompleter = Completer(); } chromeConnection.close(); _process?.kill(); diff --git a/packages/flutter_tools/test/general.shard/web/chrome_test.dart b/packages/flutter_tools/test/general.shard/web/chrome_test.dart index 3312a38d8b..30e58147f6 100644 --- a/packages/flutter_tools/test/general.shard/web/chrome_test.dart +++ b/packages/flutter_tools/test/general.shard/web/chrome_test.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'package:file/memory.dart'; +import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/os.dart'; @@ -62,28 +63,31 @@ void main() { }); test('can launch chrome and connect to the devtools', () async { - processManager.addCommand(const FakeCommand( - command: [ - 'example_chrome', - '--user-data-dir=/.tmp_rand0/flutter_tool.rand0', - '--remote-debugging-port=1234', - ..._kChromeArgs, - 'example_url', - ], - stderr: kDevtoolsStderr, - )); + await testLaunchChrome('/.tmp_rand0/flutter_tool.rand0', processManager, chromeLauncher); + }); - await chromeLauncher.launch( - 'example_url', - skipCheck: true, - ); + test('cannot have two concurrent instances of chrome', () async { + await testLaunchChrome('/.tmp_rand0/flutter_tool.rand0', processManager, chromeLauncher); + bool pass = false; + try { + await testLaunchChrome('/.tmp_rand0/flutter_tool.rand1', processManager, chromeLauncher); + } on ToolExit catch (_) { + pass = true; + } + expect(pass, isTrue); + }); + + test('can launch new chrome after stopping a previous chrome', () async { + final Chrome chrome = await testLaunchChrome('/.tmp_rand0/flutter_tool.rand0', processManager, chromeLauncher); + await chrome.close(); + await testLaunchChrome('/.tmp_rand0/flutter_tool.rand1', processManager, chromeLauncher); }); test('can launch chrome with a custom debug port', () async { processManager.addCommand(const FakeCommand( command: [ 'example_chrome', - '--user-data-dir=/.tmp_rand0/flutter_tool.rand0', + '--user-data-dir=/.tmp_rand1/flutter_tool.rand1', '--remote-debugging-port=10000', ..._kChromeArgs, 'example_url', @@ -102,7 +106,7 @@ void main() { processManager.addCommand(const FakeCommand( command: [ 'example_chrome', - '--user-data-dir=/.tmp_rand0/flutter_tool.rand0', + '--user-data-dir=/.tmp_rand1/flutter_tool.rand1', '--remote-debugging-port=1234', ..._kChromeArgs, '--headless', @@ -133,7 +137,7 @@ void main() { processManager.addCommand(FakeCommand(command: const [ 'example_chrome', - '--user-data-dir=/.tmp_rand0/flutter_tool.rand0', + '--user-data-dir=/.tmp_rand1/flutter_tool.rand1', '--remote-debugging-port=1234', ..._kChromeArgs, 'example_url', @@ -146,7 +150,7 @@ void main() { ); final File tempFile = fileSystem - .directory('.tmp_rand0/flutter_tool.rand0') + .directory('.tmp_rand1/flutter_tool.rand1') .childDirectory('Default') .childFile('preferences'); @@ -163,3 +167,21 @@ void main() { } class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {} + +Future testLaunchChrome(String userDataDir, FakeProcessManager processManager, ChromeLauncher chromeLauncher) { + processManager.addCommand(FakeCommand( + command: [ + 'example_chrome', + '--user-data-dir=$userDataDir', + '--remote-debugging-port=1234', + ..._kChromeArgs, + 'example_url', + ], + stderr: kDevtoolsStderr, + )); + + return chromeLauncher.launch( + 'example_url', + skipCheck: true, + ); +}