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

Process hangs when running multiple concurrent precompiled browser test files #2294

Closed
greglittlefield-wf opened this issue Oct 15, 2024 · 5 comments · Fixed by #2422
Closed

Comments

@greglittlefield-wf
Copy link
Contributor

Hi, I noticed that in Dart 3, when I run multiple browser test files with the default concurrency, all the tests run properly, but the build_runner and test processes never exit.

What it looks like:

% dart run build_runner test -- -p chrome test/a_test.dart test/b_test.dart
[INFO] Generating build script completed, took 226ms
...
[INFO] Succeeded after 650ms with 0 outputs (0 actions)
Running tests...

00:01 +2: All tests passed!
<<<At this point, the process continues running indefinitely>>>

I'm seeing this behavior both locally in macOS, and on GitHub Actions runs. Locally, it seems that the Chrome processes are also still open in the background.

This only seems to happen when using build_runner, but not when running tests directly from test.

It also only happens when running more than one test file, and does not occur when passing --concurrency=1 to the test command, nor does it happen when passing --pause-after-load. So, --concurrency=1 seems like a good workaround.

I also tested the --release flag, and not specifying files explicitly via the command line, and those variables didn't seem to make a difference.

I've set up a basic repo as a reduced test case here: https://github.com/greglittlefield-wf/build_runner_hang_repro

To reproduce the issue:

git clone https://github.com/greglittlefield-wf/build_runner_hang_repro.git
cd build_runner_hang_repro
dart pub get
dart run build_runner test -- -p chrome test/a_test.dart test/b_test.dart

Examples of commands that don't reproduce the issue:

# Running a single test
dart run build_runner test -- -p chrome test/a_test.dart

# Running with concurrency=1
dart run build_runner test -- -p chrome --concurrency=1 test/a_test.dart test/b_test.dart

# Running via test instead of build_runner
dart run test -p chrome test/a_test.dart test/b_test.dart

Versions combinations I've reproduced the issue under:

  • Dart SDK 3.5.3, latest test and build packages
    • build 2.4.1, build_runner 2.4.13, build_test 2.2.2, build_web_compilers 4.0.11, test 1.25.8, test_api 0.7.3, test_core 0.6.5
  • Dart SDK 3.0.0, latest resolvable test and build packages
    • build 2.4.1, build_runner 2.4.9, build_test 2.2.2, build_web_compilers 4.0.6, test 1.25.5, test_api 0.7.1, test_core 0.6.2

Versions that don't reproduce the issue:

  • Dart SDK 2.19.6, latest resolvable test and build packages
    • build 2.3.1, build_runner 2.3.3, build_test 2.2.1, build_web_compilers 3.2.7, test 1.24.3, test_api 0.6.0, test_core 0.5.3

I spent some time trying a Dart 2.19.6 pubspec.lock and running that in Dart 3, but there kept being incompatibilities in older versions of packages that prevented me from building, so I gave up.

@jakemac53
Copy link
Contributor

Interesting, I can repro it with your example, but our integration test in this repo does not have this behavior (dart run build_runner test -- -p chrome test/hello_world_test.dart test/hello_world_deferred_test.dart from _test). I can look into it.

@jakemac53
Copy link
Contributor

It definitely is package:test which isn't exiting properly here, we never get an exit code from it.

@jakemac53
Copy link
Contributor

cc @natebosch I am getting to this point https://github.com/dart-lang/test/blob/master/pkgs/test_core/lib/src/runner.dart#L161 (with success == true), but then it hangs. So, other async task seems to still be pending (likely a stream listening on some IO somewhere is still open).

I am transferring this to the test repo.

@jakemac53 jakemac53 transferred this issue from dart-lang/build Oct 17, 2024
@jakemac53 jakemac53 changed the title Process hangs when running multiple concurrent browser test files Process hangs when running multiple concurrent precompiled browser test files Oct 17, 2024
jakemac53 added a commit that referenced this issue Oct 18, 2024
This was intended as a regression test for
#2294 but it actually passes 😭
@natebosch
Copy link
Member

I have not been able to track this down yet. I have validated that this is unrelated to build_runner test using DDC or a problem with the way it bootstraps test suites. This does seem to be a problem specifically with precompiled browser tests.

When there are more than one suites ran I can see the LoadSuite for all but one of the suites stuck in the heap but I have not found any objects that have a retaining path which can point me to anything specific.

evanweible-wf added a commit to Workiva/test_html_builder that referenced this issue Nov 13, 2024
evanweible-wf added a commit to Workiva/test_html_builder that referenced this issue Nov 13, 2024
evanweible-wf added a commit to Workiva/test_html_builder that referenced this issue Nov 19, 2024
@natebosch
Copy link
Member

I think this has been broken since #2144

That commit breaks precompiled browser tests entirely, but when they are later fixed in #2170 the hang starts to repro, and in between the test runner does hang following the loading failures.

natebosch added a commit that referenced this issue Dec 4, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants