Skip to content

Conversation

srujzs
Copy link
Contributor

@srujzs srujzs commented Oct 9, 2025

We can't actually pause during execution so we shouldn't signal that we did.

We can't actually pause during execution so we shouldn't
signal that we did.
@srujzs srujzs requested review from bkonyi and jyameo October 9, 2025 23:05
@srujzs
Copy link
Contributor Author

srujzs commented Oct 9, 2025

I tried removing _currentPauseEvent entirely, but it looks like Flutter tools' testing is relying on a pause event at start, specifically from here when the isolate is created and pauseIsolatesOnStart is true. For now, I left that, modifications to that flag which also results in a new pause event being sent, and resume, where a resume event is sent.

It's not clear to me whether PauseStart and Resume should still be sent even though we don't pause. I suppose it signals on start that the isolate has started but we haven't run main yet?

@jyameo
Copy link
Contributor

jyameo commented Oct 10, 2025

I tried removing _currentPauseEvent entirely, but it looks like Flutter tools' testing is relying on a pause event at start, specifically from here when the isolate is created and pauseIsolatesOnStart is true. For now, I left that, modifications to that flag which also results in a new pause event being sent, and resume, where a resume event is sent.

It's not clear to me whether PauseStart and Resume should still be sent even though we don't pause. I suppose it signals on start that the isolate has started but we haven't run main yet?

It makes sense to me to keep the pause event when the isolate is created to maintain the expected behavior from Flutter tools.

Copy link
Contributor

@jyameo jyameo left a comment

Choose a reason for hiding this comment

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

LGTM!

@bkonyi
Copy link
Collaborator

bkonyi commented Oct 10, 2025

I tried removing _currentPauseEvent entirely, but it looks like Flutter tools' testing is relying on a pause event at start, specifically from here when the isolate is created and pauseIsolatesOnStart is true.

Can you point me to the failing test? I don't think there's any particular reason that the tool needs this event, so it could just be a test expectation that can be removed.

@srujzs
Copy link
Contributor Author

srujzs commented Oct 13, 2025

Can you point me to the failing test? I don't think there's any particular reason that the tool needs this event, so it could just be a test expectation that can be removed.

It's in the test driver itself: https://github.com/flutter/flutter/blob/0205d4e8cc24511918e0348410f34605102682c9/packages/flutter_tools/test/integration.shard/test_driver.dart#L295

I think it makes sense that it expects the event when the isolate starts, although perhaps it should be guarded by a conditional. I also think the WebSocketProxyService can handle it (as we can just wait to run main), so it makes sense to me to support it. I can upload a follow-up PR if we think we want to change that in DWDS.

@srujzs srujzs merged commit 2517aa9 into dart-lang:main Oct 13, 2025
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants