Set a debug name for test isolates#2494
Merged
liamappelbe merged 9 commits intomasterfrom May 13, 2025
Merged
Conversation
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. |
github-actions
bot
added
the
type-infra
github-actions
bot
removed
the
type-infra
natebosch
reviewed
May 13, 2025
| message, | ||
| packageConfig: await packageConfigUri, | ||
| checked: true, | ||
| debugName: 'test_main', |
Member
There was a problem hiding this comment.
Where are these surfaced?
Is there any reason to not put more detail in here? WDYT About
Suggested change
| debugName: 'test_main', | |
| debugName: 'test_suite:$uri', |
Contributor
Author
There was a problem hiding this comment.
They're surfaced in the VM service API. They're designed to be human readable names and are handy for debugging. So yeah, it definitely makes sense to include the test URI.
test_core 0.6.9 is already published, bump to a -wip version.
natebosch
approved these changes
May 13, 2025
copybara-service bot
pushed a commit
to dart-lang/sdk
that referenced
this pull request
May 17, 2025
Revisions updated by `dart tools/rev_sdk_deps.dart`. dartdoc (https://github.com/dart-lang/dartdoc/compare/95f4208..e38f392): e38f3921 2025-05-16 Sigurd Meldgaard Remove analyzer_use_new_elements from analysis_options.yaml (dart-lang/dartdoc#4050) protobuf (https://github.com/dart-lang/protobuf/compare/9bd149b..b7753f6): b7753f6 2025-05-16 Devon Carew rev package:protobuf version (google/protobuf.dart#994) b5d20ff 2025-05-16 Ömer Sinan Ağacan Update protobuf changelog with `#981` (google/protobuf.dart#995) test (https://github.com/dart-lang/test/compare/55d1f9e..b9c59ea): b9c59ea0 2025-05-13 Liam Appelbe Set a debug name for test isolates (dart-lang/test#2494) a1e295b4 2025-05-13 Liam Appelbe Fix CI 3c3878af 2025-05-13 Liam Appelbe Include the test URI in the debug name 90e64ec2 2025-05-13 Nate Bosch Bump version d67c897b 2025-05-13 Liam Appelbe Merge branch 'master' into isolate_debug_name e6d4877e 2025-05-12 Jacob MacDonald release test packages (dart-lang/test#2495) 4097e1be 2025-05-07 Liam Appelbe revert workflow debugging 7800c010 2025-05-07 Liam Appelbe fmt 455483b5 2025-05-07 Liam Appelbe changelog c9b5b6fa 2025-05-07 Liam Appelbe fmt 39c4b31d 2025-05-07 Liam Appelbe Set a debug name for test isolates vector_math (https://github.com/google/vector_math.dart/compare/0279cb8..13f185f): 13f185f 2025-05-16 Kevin Moore Remove prefer-inline for non-trivial Matrix functions (google/vector_math.dart#347) webdev (https://github.com/dart-lang/webdev/compare/1ea8462..5dbb30e): 5dbb30eb 2025-05-12 Jessy Yameogo Support hot reload over websocket (dart-lang/webdev#2616) Change-Id: I85b001857d0864bd50390d82aa142938a4c530d4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428927 Commit-Queue: Devon Carew <devoncarew@google.com> Reviewed-by: Kevin Moore <kevmoo@google.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Dart VM treats the main isolate specially. When the main isolate shuts down, the entire VM shuts down, even if there are other isolates still running.
package:coverage needs to collect coverage for all isolates. To do this, the VM is run with
--pause-isolates-on-exit, and package:coverage waits for each isolate to pause, collects the coverage, then resumes the isolate and allows it to exit. But since the main isolate exiting would cause all isolates to exit, some of which may not have been collected yet, package:coverage needs to treat the main isolate specially, and avoid resuming it until every other isolate is collected.Unfortunately there's no reliable way of determining which isolate is the main isolate using the VM service API. So package:coverage uses a heuristic: the first isolate is sees with a debug name of "main" is the main isolate. 99% of the time this works. But package:test spawns a new isolate for each test file, and doesn't set a debug name, so the name defaults to "main". If one of those test isolates is seen by package:coverage before the true main isolate, the system breaks. This causes about a 1% flakiness for test coverage in g3.
We're working on extending the VM service API to tell us which isolate is the main one, but it's turning out to be non-trivial. In the meantime, a quick fix for the g3 flakiness is just to set some other debug name on the isolates that package:test spawns.
I've manually tested that this works, but let me know if I missed any isolate spawns that could be hit during
dart test. Also, do you think this needs a test?See also: dart-lang/sdk#56732