Fix hang after multiple precompiled browser tests (#2422)

Fixes #2294

Avoid creating extra unexpected BrowserManager instances by caching the
future in the `_browserManagers` map without any async delay. Previously
it was possible for two managers to be created if the second suite is
loaded before the first suite's `compilerSupport` was resolved. This was
not a problem for tests that get compiled by the test runner because the
compilation would delay the second suite load until after the first
suite's `compilerSupport` has resolved. It is not a problem when running
without concurrency because that delays the second suite load.

Add a concurrency argument to the regression test, otherwise the default
is to run with concurrency 1 which works around the bug.

Author: natebosch

pkgs/test/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.25.12
+
+* Fix hang when running multiple precompiled browser tests.
+
 ## 1.25.11
 
 * Update to be forward compatible with `package:shelf_web_socket` version `3.x`.

pkgs/test/lib/src/runner/browser/platform.dart

@@ -211,10 +211,21 @@ class BrowserPlatform extends PlatformPlugin
   ///
   /// If no browser manager is running yet, starts one.
   Future<BrowserManager?> _browserManagerFor(
-      Runtime browser, Compiler compiler) async {
+      Runtime browser, Compiler compiler) {
     var managerFuture = _browserManagers[(browser, compiler)];
     if (managerFuture != null) return managerFuture;
 
+    var future = _createBrowserManager(browser, compiler);
+    // Store null values for browsers that error out so we know not to load them
+    // again.
+    _browserManagers[(browser, compiler)] =
+        future.then<BrowserManager?>((value) => value).onError((_, __) => null);
+
+    return future;
+  }
+
+  Future<BrowserManager> _createBrowserManager(
+      Runtime browser, Compiler compiler) async {
     var support = await compilerSupport(compiler);
     var (webSocketUrl, socketFuture) = support.webSocket;
     var hostUrl = support.serverUrl
@@ -224,15 +235,8 @@ class BrowserPlatform extends PlatformPlugin
       'debug': _config.debug.toString()
     });
 
-    var future = BrowserManager.start(
+    return BrowserManager.start(
         browser, hostUrl, socketFuture, _browserSettings[browser]!, _config);
-
-    // Store null values for browsers that error out so we know not to load them
-    // again.
-    _browserManagers[(browser, compiler)] =
-        future.then<BrowserManager?>((value) => value).onError((_, __) => null);
-
-    return future;
   }
 
   /// Close all the browsers that the server currently has open.

pkgs/test/pubspec.yaml

@@ -1,5 +1,5 @@
 name: test
-version: 1.25.11
+version: 1.25.12
 description: >-
   A full featured library for writing and running Dart tests across platforms.
 repository: https://github.com/dart-lang/test/tree/master/pkgs/test
@@ -36,7 +36,7 @@ dependencies:
 
   # Use an exact version until the test_api and test_core package are stable.
   test_api: 0.7.4
-  test_core: 0.6.7
+  test_core: 0.6.8
 
   typed_data: ^1.3.0
   web_socket_channel: '>=2.0.0 <4.0.0'

pkgs/test/test/runner/precompiled_test.dart

@@ -42,7 +42,7 @@ void main() {
 
     test('run two precompiled tests', () async {
       await _precompileBrowserTest('test_2.dart');
-      var test = await runTest([
+      var test = await runTest(concurrency: 2, [
         '-p',
         'chrome',
         '--precompiled=precompiled/',

pkgs/test_core/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.6.8
+
+* Fix hang when running multiple precompiled browser tests.
+
 ## 0.6.7
 
 * Update the `package:vm_service` constraint to allow version `15.x`.

pkgs/test_core/pubspec.yaml

@@ -1,5 +1,5 @@
 name: test_core
-version: 0.6.7
+version: 0.6.8
 description: A basic library for writing tests and running them on the VM.
 repository: https://github.com/dart-lang/test/tree/master/pkgs/test_core
 resolution: workspace