Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hang after multiple precompiled browser tests #2422

Merged
merged 1 commit into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Fix hang after multiple precompiled browser tests
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.
  • Loading branch information
natebosch committed Dec 4, 2024
commit 69bbc318bb372522b5fdd489d325e43d0648f3fc
4 changes: 4 additions & 0 deletions pkgs/test/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down
22 changes: 13 additions & 9 deletions pkgs/test/lib/src/runner/browser/platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions pkgs/test/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion pkgs/test/test/runner/precompiled_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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/',
Expand Down
4 changes: 4 additions & 0 deletions pkgs/test_core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down
2 changes: 1 addition & 1 deletion pkgs/test_core/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading