Skip to content

Commit

Permalink
Add some retry around browser launching (#1676)
Browse files Browse the repository at this point in the history
Towards #1578

Recursively attempt to launch the browser on the first few timeouts
before throwing the ApplicationException.

Restore previous timeout of 30 seconds.
The 45 second timeout was mainly to investigate whether extra time would
resolve flakes. It didn't seem to help at all and it doesn't seem likely
that even more time would help. Go back to 30 seconds to reduce the pain
of hitting retries (though it will still be painful).
  • Loading branch information
natebosch committed Mar 16, 2022
1 parent 5d701cb commit b6aba55
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 7 deletions.
3 changes: 2 additions & 1 deletion pkgs/test/CHANGELOG.md
@@ -1,7 +1,8 @@
## 1.20.2-dev
## 1.20.2

* Drop `dart2js-path` command line argument.
* Allow loading tests under a path with the directory named `packages`.
* Add retry for launching browsers. Reduce timeout back to 30 seconds.

## 1.20.1

Expand Down
23 changes: 18 additions & 5 deletions pkgs/test/lib/src/runner/browser/browser_manager.dart
Expand Up @@ -98,11 +98,21 @@ class BrowserManager {
/// Returns the browser manager, or throws an [ApplicationException] if a
/// connection fails to be established.
static Future<BrowserManager> start(
Runtime runtime,
Uri url,
Future<WebSocketChannel> future,
ExecutableSettings settings,
Configuration configuration) =>
_start(runtime, url, future, settings, configuration, 1);

static const _maxRetries = 3;
static Future<BrowserManager> _start(
Runtime runtime,
Uri url,
Future<WebSocketChannel> future,
ExecutableSettings settings,
Configuration configuration) {
Configuration configuration,
int attempt) {
var browser = _newBrowser(url, runtime, settings, configuration);

var completer = Completer<BrowserManager>();
Expand All @@ -127,11 +137,14 @@ class BrowserManager {
completer.completeError(error, stackTrace);
});

return completer.future.timeout(Duration(seconds: 45), onTimeout: () {
return completer.future.timeout(Duration(seconds: 30), onTimeout: () {
browser.close();
throw ApplicationException(
'Timed out waiting for ${runtime.name} to connect.\n'
'Browser output: ${utf8.decode(browser.output)}');
if (attempt >= _maxRetries) {
throw ApplicationException(
'Timed out waiting for ${runtime.name} to connect.\n'
'Browser output: ${utf8.decode(browser.output)}');
}
return _start(runtime, url, future, settings, configuration, ++attempt);
});
}

Expand Down
2 changes: 1 addition & 1 deletion pkgs/test/pubspec.yaml
@@ -1,5 +1,5 @@
name: test
version: 1.20.2-dev
version: 1.20.2
description: >-
A full featured library for writing and running Dart tests across platforms.
repository: https://github.com/dart-lang/test/blob/master/pkgs/test
Expand Down

0 comments on commit b6aba55

Please sign in to comment.