Fix web test flakiness; enable web golden tests (#52789)
* Fix web test flakiness; enable web golden tests
The fix is three-part:
- Only allow one test to load _and_ test at any point in time.
- Use a fresh Chrome instance for each test file.
- Increase Cirrus resources.
The first two changes only fix the "Unknown error loading" error, but not hanging tests. The resource increase also prevents hanging tests.
Other minor changes:
- Remove test batching (it's no longer necessary)
- Fix the Chrome class, which was using the wrong Completer.
diff --git a/.cirrus.yml b/.cirrus.yml
index 286e8a6..1d37d5f 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 32efaec..37ff25b 100644
--- a/dev/bots/test.dart
+++ b/dev/bots/test.dart
@@ -53,11 +53,6 @@
/// 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<void> _runFlutterWebTest(String workingDirectory, List<String> tests) async {
- final List<String> batch = <String>[];
- 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,
- <String>[
- 'test',
- if (ciProvider == CiProviders.cirrus)
- '--concurrency=1', // do not parallelize on Cirrus, to reduce flakiness
- '-v',
- '--platform=chrome',
- ...?flutterTestArgs,
- ...batch,
- ],
- workingDirectory: workingDirectory,
- environment: <String, String>{
- 'FLUTTER_WEB': 'true',
- 'FLUTTER_LOW_RESOURCE_MODE': 'true',
- },
- );
- batch.clear();
- }
- }
+ await runCommand(
+ flutter,
+ <String>[
+ 'test',
+ if (ciProvider == CiProviders.cirrus)
+ '--concurrency=1', // do not parallelize on Cirrus, to reduce flakiness
+ '-v',
+ '--platform=chrome',
+ ...?flutterTestArgs,
+ ...tests,
+ ],
+ workingDirectory: workingDirectory,
+ environment: <String, String>{
+ 'FLUTTER_WEB': 'true',
+ 'FLUTTER_LOW_RESOURCE_MODE': 'true',
+ },
+ );
}
Future<void> _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 aad4876..d3aa222 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 @@
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 f17de80..793611e 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 @@
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 @@
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 @@
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 @@
..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 @@
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 fbbc3a9..a720435 100644
--- a/packages/flutter/test/material/dialog_theme_test.dart
+++ b/packages/flutter/test/material/dialog_theme_test.dart
@@ -132,7 +132,7 @@
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 5b2d265..08ddf9f 100644
--- a/packages/flutter/test/material/dropdown_test.dart
+++ b/packages/flutter/test/material/dropdown_test.dart
@@ -189,7 +189,7 @@
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 @@
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 8188d6e..09077fe 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 @@
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 472403a..272512c 100644
--- a/packages/flutter/test/material/radio_test.dart
+++ b/packages/flutter/test/material/radio_test.dart
@@ -282,7 +282,7 @@
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 c2aebf0..cbdd97a 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 @@
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 @@
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 @@
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 @@
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 0c3995a..f3eb9f7 100644
--- a/packages/flutter/test/rendering/localized_fonts_test.dart
+++ b/packages/flutter/test/rendering/localized_fonts_test.dart
@@ -52,7 +52,6 @@
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 6bfcbd9..6d25448 100644
--- a/packages/flutter/test/widgets/backdrop_filter_test.dart
+++ b/packages/flutter/test/widgets/backdrop_filter_test.dart
@@ -45,5 +45,5 @@
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 94f1924..2136bb4 100644
--- a/packages/flutter/test/widgets/clip_test.dart
+++ b/packages/flutter/test/widgets/clip_test.dart
@@ -383,7 +383,7 @@
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 @@
find.byType(RepaintBoundary).first,
matchesGoldenFile('clip.ClipRectOverlay.png'),
);
- }, skip: isBrowser);
+ });
testWidgets('ClipRRect painting', (WidgetTester tester) async {
await tester.pumpWidget(
@@ -472,7 +472,7 @@
find.byType(RepaintBoundary).first,
matchesGoldenFile('clip.ClipRRect.png'),
);
- }, skip: isBrowser);
+ });
testWidgets('ClipOval painting', (WidgetTester tester) async {
await tester.pumpWidget(
@@ -515,7 +515,7 @@
find.byType(RepaintBoundary).first,
matchesGoldenFile('clip.ClipOval.png'),
);
- }, skip: isBrowser);
+ });
testWidgets('ClipPath painting', (WidgetTester tester) async {
await tester.pumpWidget(
@@ -563,7 +563,7 @@
find.byType(RepaintBoundary).first,
matchesGoldenFile('clip.ClipPath.png'),
);
- }, skip: isBrowser);
+ });
Center genPhysicalModel(Clip clipBehavior) {
return Center(
@@ -608,7 +608,7 @@
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 @@
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 @@
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 @@
find.byType(RepaintBoundary).first,
matchesGoldenFile('clip.PhysicalModel.default.png'),
);
- }, skip: isBrowser);
+ });
Center genPhysicalShape(Clip clipBehavior) {
return Center(
@@ -717,7 +717,7 @@
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 @@
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 @@
find.byType(RepaintBoundary).first,
matchesGoldenFile('clip.PhysicalShape.antiAliasWithSaveLayer.png'),
);
- }, skip: isBrowser);
+ });
testWidgets('PhysicalShape painting', (WidgetTester tester) async {
await tester.pumpWidget(
@@ -779,7 +779,7 @@
find.byType(RepaintBoundary).first,
matchesGoldenFile('clip.PhysicalShape.default.png'),
);
- }, skip: isBrowser);
+ });
testWidgets('ClipPath.shape', (WidgetTester tester) async {
final List<String> logs = <String>[];
diff --git a/packages/flutter/test/widgets/invert_colors_test.dart b/packages/flutter/test/widgets/invert_colors_test.dart
index fc41648..f899d60 100644
--- a/packages/flutter/test/widgets/invert_colors_test.dart
+++ b/packages/flutter/test/widgets/invert_colors_test.dart
@@ -22,7 +22,7 @@
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 @@
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 0d86ddb..4de089f 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 @@
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 @@
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 bda72f3..46e3ca4 100644
--- a/packages/flutter/test/widgets/opacity_test.dart
+++ b/packages/flutter/test/widgets/opacity_test.dart
@@ -180,7 +180,7 @@
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 0780f5a..13fc561 100644
--- a/packages/flutter/test/widgets/physical_model_test.dart
+++ b/packages/flutter/test/widgets/physical_model_test.dart
@@ -112,7 +112,7 @@
find.byKey(key),
matchesGoldenFile('physical_model_overflow.png'),
);
- }, skip: isBrowser);
+ });
group('PhysicalModelLayer checks elevation', () {
Future<void> _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 9434a45..de39ee8 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 @@
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', <String, Object>{
@@ -303,10 +301,7 @@
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<Runtime, Future<BrowserManager>> _browserManagers =
- <Runtime, Future<BrowserManager>>{};
+ BrowserManager _browserManager;
// A handler that serves wrapper files used to bootstrap tests.
shelf.Response _wrapperHandler(shelf.Request request) {
@@ -330,6 +325,11 @@
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<RunnerSuite> load(
String path,
@@ -340,17 +340,28 @@
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 @@
/// Returns the [BrowserManager] for [runtime], which should be a browser.
///
/// If no browser manager is running yet, starts one.
- Future<BrowserManager> _browserManagerFor(Runtime browser) {
- final Future<BrowserManager> managerFuture = _browserManagers[browser];
- if (managerFuture != null) {
- return managerFuture;
+ Future<BrowserManager> _launchBrowser(Runtime browser) {
+ if (_browserManager != null) {
+ throw StateError('Another browser is currently running.');
}
+
final Completer<WebSocketChannel> completer =
Completer<WebSocketChannel>.sync();
final String path =
@@ -383,48 +394,29 @@
globals.printTrace('Serving tests at $hostUrl');
- final Future<BrowserManager> 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<void> closeEphemeral() {
- final List<Future<BrowserManager>> managers =
- _browserManagers.values.toList();
- _browserManagers.clear();
- return Future.wait(managers.map((Future<BrowserManager> manager) async {
- final BrowserManager result = await manager;
- if (result == null) {
- return;
- }
- await result.close();
- }));
+ Future<void> closeEphemeral() async {
+ if (_browserManager != null) {
+ await _browserManager.close();
+ }
}
@override
Future<void> close() => _closeMemo.runOnce(() async {
- final List<Future<dynamic>> futures = _browserManagers.values
- .map<Future<dynamic>>((Future<BrowserManager> 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<void>(futures);
+ await Future.wait<void>(<Future<dynamic>>[
+ if (_browserManager != null)
+ _browserManager.close(),
+ _server.close(),
+ _testGoldenComparator.close(),
+ ]);
});
}
@@ -578,21 +570,6 @@
/// This is connected to a page running `static/host.dart`.
MultiChannel<dynamic> _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 @@
final Completer<BrowserManager> completer = Completer<BrowserManager>();
- 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 @@
String path,
Uri url,
SuiteConfiguration suiteConfig,
- Object message,
+ Object message, {
+ Future<void> Function() onDone,
+ }
) async {
url = url.replace(fragment: Uri.encodeFull(jsonEncode(<String, Object>{
'metadata': suiteConfig.metadata.serialize(),
@@ -726,29 +705,28 @@
StreamTransformer<dynamic, dynamic>.fromHandlers(handleDone: (EventSink<dynamic> sink) {
closeIframe();
sink.close();
+ onDone();
}),
);
- return await _pool.withResource<RunnerSuite>(() async {
- _channel.sink.add(<String, Object>{
- '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(<String, Object>{
+ '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 55a998b..e28b3be 100644
--- a/packages/flutter_tools/lib/src/web/chrome.dart
+++ b/packages/flutter_tools/lib/src/web/chrome.dart
@@ -116,6 +116,10 @@
///
/// `skipCheck` does not attempt to make a devtools connection before returning.
Future<Chrome> 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 @@
}
static Future<Chrome> _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 @@
final ChromeConnection chromeConnection;
final Uri remoteDebuggerUri;
- static Completer<Chrome> _currentCompleter = Completer<Chrome>();
-
- Future<void> get onExit => _currentCompleter.future;
+ Future<int> get onExit => _process.exitCode;
Future<void> close() async {
- if (_currentCompleter.isCompleted) {
- _currentCompleter = Completer<Chrome>();
+ if (ChromeLauncher.hasChromeInstance) {
+ ChromeLauncher._currentCompleter = Completer<Chrome>();
}
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 3312a38..30e5814 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 @@
});
test('can launch chrome and connect to the devtools', () async {
- processManager.addCommand(const FakeCommand(
- command: <String>[
- '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: <String>[
'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 @@
processManager.addCommand(const FakeCommand(
command: <String>[
'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 @@
processManager.addCommand(FakeCommand(command: const <String>[
'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 @@
);
final File tempFile = fileSystem
- .directory('.tmp_rand0/flutter_tool.rand0')
+ .directory('.tmp_rand1/flutter_tool.rand1')
.childDirectory('Default')
.childFile('preferences');
@@ -163,3 +167,21 @@
}
class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {}
+
+Future<Chrome> testLaunchChrome(String userDataDir, FakeProcessManager processManager, ChromeLauncher chromeLauncher) {
+ processManager.addCommand(FakeCommand(
+ command: <String>[
+ 'example_chrome',
+ '--user-data-dir=$userDataDir',
+ '--remote-debugging-port=1234',
+ ..._kChromeArgs,
+ 'example_url',
+ ],
+ stderr: kDevtoolsStderr,
+ ));
+
+ return chromeLauncher.launch(
+ 'example_url',
+ skipCheck: true,
+ );
+}
