From dc0f8ea4d09aabb0fed16daea3d4653c8f967b02 Mon Sep 17 00:00:00 2001
From: Nate Bosch <nbosch@google.com>
Date: Wed, 4 Dec 2024 08:54:38 -0800
Subject: [PATCH] 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.
---
 pkgs/test/CHANGELOG.md                        |  4 ++++
 .../test/lib/src/runner/browser/platform.dart | 22 +++++++++++--------
 pkgs/test/pubspec.yaml                        |  4 ++--
 pkgs/test/test/runner/precompiled_test.dart   |  2 +-
 pkgs/test_core/CHANGELOG.md                   |  4 ++++
 pkgs/test_core/pubspec.yaml                   |  2 +-
 6 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md
index c200b2189..afe8dec19 100644
--- a/pkgs/test/CHANGELOG.md
+++ b/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`.
diff --git a/pkgs/test/lib/src/runner/browser/platform.dart b/pkgs/test/lib/src/runner/browser/platform.dart
index c4df1b32c..da03a3e00 100644
--- a/pkgs/test/lib/src/runner/browser/platform.dart
+++ b/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.
diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml
index a66e59271..0ab65ad40 100644
--- a/pkgs/test/pubspec.yaml
+++ b/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'
diff --git a/pkgs/test/test/runner/precompiled_test.dart b/pkgs/test/test/runner/precompiled_test.dart
index 8eb135d55..51dde28b9 100644
--- a/pkgs/test/test/runner/precompiled_test.dart
+++ b/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/',
diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md
index 321f01952..4c305bc61 100644
--- a/pkgs/test_core/CHANGELOG.md
+++ b/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`.
diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml
index b606bb3d0..d08e24ca9 100644
--- a/pkgs/test_core/pubspec.yaml
+++ b/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