Skip to content

Fix auto select main isolate#9756

Open
rishika0212 wants to merge 6 commits intoflutter:masterfrom
rishika0212:fix-auto-select-main-isolate
Open

Fix auto select main isolate#9756
rishika0212 wants to merge 6 commits intoflutter:masterfrom
rishika0212:fix-auto-select-main-isolate

Conversation

@rishika0212
Copy link
Copy Markdown
Contributor

@rishika0212 rishika0212 commented Apr 3, 2026

Fixes #9747

When DevTools connects to a test run, three isolates are present: the test runner (main), the test suite isolate where user code actually runs (test_suite:...), and vm-service. Previously, DevTools fell through to selecting the first isolate (main) as a fallback, so CPU profiling and other tools showed no data and the user's test code was running in a completely different isolate.

This changes _computeMainIsolate in IsolateManager to prefer thetest_suite: isolate when no Flutter extension or :main( isolate is found. The test_suite: prefix is set as a debugName by package:test_core when it spawns each test suite isolate, making it a reliable signal to detect test runs.

Before: CPU profiler showed empty results when connected to a test run.
After: CPU profiler correctly captures samples from the test suite isolate.

Pre-launch Checklist

General checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read the [Flutter Style Guide] recently, and have followed its advice.
  • I signed the [CLA].
  • I updated/added relevant documentation (doc comments with ///).

Issues checklist

  • I listed at least one issue that this PR fixes in the description above.
  • I listed at least one issue which has the [contributions-welcome] or [good-first-issue] label.
  • I did not list at least one issue with the [contributions-welcome] or [good-first-issue] label. I understand this means my PR might take longer to be reviewed.

Tests checklist

  • I added new tests to check the change I am making...
  • OR there is a reason for not adding tests, which I explained in the PR description.

AI-tooling checklist

  • I did not use any AI tooling in creating this PR.
  • OR I did use AI tooling, and...
    • I read the [AI contributions guidelines] and agree to follow them.
    • I reviewed all AI-generated code before opening this PR.
    • I understand and am able to discuss the code in this PR.
    • I have verifed the accuracy of any AI-generated text included in the PR description.
    • I commit to verifying the accuracy of any AI-generated code or text that I upload in response to review comments.

Feature-change checklist

  • This PR does not change the DevTools UI or behavior and...
    • I added the release-notes-not-required label or left a comment requesting the label be added.
  • OR this PR does change the DevTools UI or behavior and...
    • I added an entry to packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.
    • I included before/after screenshots and/or a GIF demo of the new UI to my PR description.
    • I ran the DevTools app locally to manually verify my changes.

@rishika0212 rishika0212 requested review from a team and kenzieschmoll as code owners April 3, 2026 11:00
@rishika0212 rishika0212 requested review from bkonyi and removed request for a team April 3, 2026 11:00
@kenzieschmoll kenzieschmoll requested a review from jakemac53 April 9, 2026 17:45
@kenzieschmoll
Copy link
Copy Markdown
Member

kenzieschmoll commented Apr 9, 2026

LGTM from me. Will wait for @bkonyi and @jakemac53 to review before landing.

@jakemac53
Copy link
Copy Markdown
Contributor

jakemac53 commented Apr 10, 2026 •
edited
Loading

In general I don't love the idea of hard coding an assumption about the test isolate debug names - although there probably isn't a better solution really.

What I would do is make a PR to package:test which adds some comments where this is set up, that changing it will break DevTools.

jakemac53
jakemac53 approved these changes Apr 10, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jakemac53 jakemac53 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if we do the additional PR to add comments into package:test about not changing the debug name

jakemac53
jakemac53 reviewed Apr 10, 2026
View reviewed changes
@rishika0212
Copy link
Copy Markdown
Contributor Author

rishika0212 commented Apr 10, 2026 •
edited
Loading

In general I don't love the idea of hard coding an assumption about the test isolate debug names - although there probably isn't a better solution really.

What I would do is make a PR to package:test which adds some comments where this is set up, that changing it will break DevTools.

Will do
I'll open a follow-up PR to package:test

bkonyi
bkonyi reviewed Apr 10, 2026
View reviewed changes
packages/devtools_app_shared/lib/src/service/isolate_manager.dart
// isolate is where user code actually runs.
// See: https://github.com/flutter/devtools/issues/9747
final testSuiteRef = _isolateStates.keys.firstWhereOrNull(
(IsolateRef ref) => ref.name?.startsWith('test_suite:') ?? false,
Copy link
Copy Markdown
Contributor

@bkonyi bkonyi Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this isolate not marked as a system isolate? Ideally it just wouldn't appear in the isolates list.

Copy link
Copy Markdown
Contributor

@jakemac53 jakemac53 Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual test, so it's the thing we do want to find. I don't know if we can make the root isolate (test runner) be not marked as a system isolate. But that is the one we would want to ignore if any probably.

Copy link
Copy Markdown
Contributor

@bkonyi bkonyi Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that makes sense. If we mark the main isolate as a system isolate, I'm not sure we'll need this change. We already do it for package:test via dart test, so we should probably make a similar change in flutter_tester to set the main isolate as a system isolate.

Copy link
Copy Markdown
Contributor

@jakemac53 jakemac53 Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can achieve the same result that way it would definitely be preferable to matching based on debug name and other heuristics

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jakemac53 jakemac53 jakemac53 approved these changes

@bkonyi bkonyi bkonyi left review comments

@kenzieschmoll kenzieschmoll kenzieschmoll approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Automatically select main isolate instead of the test runner isolate when connecting to test runs

4 participants

@rishika0212 @kenzieschmoll @jakemac53 @bkonyi