From b6aba5544628730b7d6a38eae1aef9117a1bb235 Mon Sep 17 00:00:00 2001
From: Nate Bosch <nbosch@google.com>
Date: Wed, 16 Mar 2022 14:46:07 -0700
Subject: [PATCH] Add some retry around browser launching (#1676)

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).
---
 pkgs/test/CHANGELOG.md                        |  3 ++-
 .../src/runner/browser/browser_manager.dart   | 23 +++++++++++++++----
 pkgs/test/pubspec.yaml                        |  2 +-
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md
index 8aea7798b..11faa50c9 100644
--- a/pkgs/test/CHANGELOG.md
+++ b/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
 
diff --git a/pkgs/test/lib/src/runner/browser/browser_manager.dart b/pkgs/test/lib/src/runner/browser/browser_manager.dart
index 5183cf8c4..23c601099 100644
--- a/pkgs/test/lib/src/runner/browser/browser_manager.dart
+++ b/pkgs/test/lib/src/runner/browser/browser_manager.dart
@@ -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>();
@@ -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);
     });
   }
 
diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml
index 1f3123288..237dd4133 100644
--- a/pkgs/test/pubspec.yaml
+++ b/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