[flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during terminate #108310

DanTup · 2022-07-25T18:15:22Z

Fixes an issue where the flutter_tester device may not be cleaned up correctly if we just terminate the Flutter process in the DAP when the client calls terminateRequest. This calls the app.stop/app.detach commands first and waits for them to complete (or the process to terminate).

Pre-launch 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 and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…minate Fixes an issue where the flutter_tester device may not be cleaned up correctly if we just terminate the Flutter process.

christopherfujino

LGTM

christopherfujino · 2022-07-25T18:43:12Z

Looks like you'll need to update expectations for the integration test too:

10:14 +28 -2: test/integration.shard/debug_adapter/flutter_adapter_test.dart: launch can run and terminate a Flutter app in debug mode [E]                                                             
  Expected: [
              'Launching lib/main.dart on Flutter test device in debug mode...',
              <a string starting with 'Connecting to VM Service at'>,
              'topLevelFunction',
              '',
              <a string starting with 'Exited'>
            ]
    Actual: [
              'Launching lib/main.dart on Flutter test device in debug mode...',
              'Connecting to VM Service at ws://127.0.0.1:42277/c2gksXLXhog=/ws',
              'topLevelFunction',
              'Application finished.',
              '',
              'Exited.'
            ]
     Which: at location [3] is 'Application finished.' instead of ''
  
  package:test_api                                                     expect
  test/integration.shard/debug_adapter/test_support.dart 49:5          expectLines
  test/integration.shard/debug_adapter/flutter_adapter_test.dart 63:7  main.<fn>.<fn>

DanTup · 2022-07-25T18:56:51Z

Oops, I think I didn't rebuild flutter_tools when I ran those (they spawn flutter debug_adapter) 🙈

Should be good now, thanks!

christopherfujino · 2022-07-25T19:02:41Z

Oops, I think I didn't rebuild flutter_tools when I ran those (they spawn flutter debug_adapter) 🙈

Should be good now, thanks!

Ahh yeah, I've hit that tricky gotcha.

DanTup · 2022-07-25T20:01:58Z

Ahh yeah, I've hit that tricky gotcha.

Embarrassingly, I'd set up a way to run the tests in-process specifically because of this, and I had to rebuild when doing the manual testing with Dart-Code's integration tests to verify it solved the original issue 🙃

(although perhaps committing temporary code to disable verbose logging is more embarrassing... also fixed now)

…ring terminate (flutter/flutter#108310)

* 2142b2e Roll Flutter Engine from cd7e2ec4037b to c31036f2381c (1 revision) (flutter/flutter#108299) * 178e444 Revert "Add optional flag to determine assertiveness level in aria announcement for flutter web" (flutter/flutter#108262) * 8a7b359 flutter update-packages --force-upgrade + analyzer fix (flutter/flutter#108198) * b3f5d3f Roll Flutter Engine from c31036f2381c to 54b0ac3059bf (1 revision) (flutter/flutter#108302) * 2f4299a [flutter_tools] Remove unused parameter when connected DAP to VM Service (flutter/flutter#108285) * e74b9b5 Migrate InputDecorator to Material 3 (flutter/flutter#107943) * e3b851a Fix Tamil DateTime representation of AM/PM (flutter/flutter#108185) * 43d6e2b Roll Flutter Engine from 54b0ac3059bf to 705072522c00 (4 revisions) (flutter/flutter#108308) * 6929ad1 Roll Flutter Engine from 705072522c00 to 3964cf62cdf8 (1 revision) (flutter/flutter#108316) * 5f67b47 [iOS] Update template icons (flutter/flutter#107873) * 606954d Added iconSize parameter in ButtonStyle (flutter/flutter#108268) * b3814c7 Roll Flutter Engine from 3964cf62cdf8 to 7f8925b1f6f3 (2 revisions) (flutter/flutter#108320) * ca6cecf Upgrade Gradle and AGP versions to 7.5/7.2 and migrate examples/tests (flutter/flutter#108197) * d155bc1 Revert "Upgrade Gradle and AGP versions to 7.5/7.2 and migrate examples/tests (#108197)" (flutter/flutter#108349) * 4056d3f Explain the "patching" protocol in `KeyMessageManager.keyMessageHandler` and add an example (flutter/flutter#105280) * b035ef1 [flutter_tools] Remove more shuffles (flutter/flutter#107759) * be14858 Roll Flutter Engine from 7f8925b1f6f3 to 8eca26d130a2 (1 revision) (flutter/flutter#108326) * 3eb638f Roll Flutter Engine from 8eca26d130a2 to f1f9b4de82b6 (5 revisions) (flutter/flutter#108350) * 401b556 Roll Flutter Engine from f1f9b4de82b6 to 11d927ac3e9b (2 revisions) (flutter/flutter#108353) * 925bee9 Roll Flutter Engine from 11d927ac3e9b to 5dcaeae6561b (1 revision) (flutter/flutter#108356) * 5d31b07 [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during terminate (flutter/flutter#108310) * c8b5d10 Roll Flutter Engine from 5dcaeae6561b to 89e117b89c63 (1 revision) (flutter/flutter#108367) * e3d08fb Hide the debug banner in the PopupMenuButton example (flutter/flutter#108324)

…minate (flutter#108310) * [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during terminate Fixes an issue where the flutter_tester device may not be cleaned up correctly if we just terminate the Flutter process. * Update integration test expectations * Revert accidental commit

* 2142b2e Roll Flutter Engine from cd7e2ec4037b to c31036f2381c (1 revision) (flutter/flutter#108299) * 178e444 Revert "Add optional flag to determine assertiveness level in aria announcement for flutter web" (flutter/flutter#108262) * 8a7b359 flutter update-packages --force-upgrade + analyzer fix (flutter/flutter#108198) * b3f5d3f Roll Flutter Engine from c31036f2381c to 54b0ac3059bf (1 revision) (flutter/flutter#108302) * 2f4299a [flutter_tools] Remove unused parameter when connected DAP to VM Service (flutter/flutter#108285) * e74b9b5 Migrate InputDecorator to Material 3 (flutter/flutter#107943) * e3b851a Fix Tamil DateTime representation of AM/PM (flutter/flutter#108185) * 43d6e2b Roll Flutter Engine from 54b0ac3059bf to 705072522c00 (4 revisions) (flutter/flutter#108308) * 6929ad1 Roll Flutter Engine from 705072522c00 to 3964cf62cdf8 (1 revision) (flutter/flutter#108316) * 5f67b47 [iOS] Update template icons (flutter/flutter#107873) * 606954d Added iconSize parameter in ButtonStyle (flutter/flutter#108268) * b3814c7 Roll Flutter Engine from 3964cf62cdf8 to 7f8925b1f6f3 (2 revisions) (flutter/flutter#108320) * ca6cecf Upgrade Gradle and AGP versions to 7.5/7.2 and migrate examples/tests (flutter/flutter#108197) * d155bc1 Revert "Upgrade Gradle and AGP versions to 7.5/7.2 and migrate examples/tests (#108197)" (flutter/flutter#108349) * 4056d3f Explain the "patching" protocol in `KeyMessageManager.keyMessageHandler` and add an example (flutter/flutter#105280) * b035ef1 [flutter_tools] Remove more shuffles (flutter/flutter#107759) * be14858 Roll Flutter Engine from 7f8925b1f6f3 to 8eca26d130a2 (1 revision) (flutter/flutter#108326) * 3eb638f Roll Flutter Engine from 8eca26d130a2 to f1f9b4de82b6 (5 revisions) (flutter/flutter#108350) * 401b556 Roll Flutter Engine from f1f9b4de82b6 to 11d927ac3e9b (2 revisions) (flutter/flutter#108353) * 925bee9 Roll Flutter Engine from 11d927ac3e9b to 5dcaeae6561b (1 revision) (flutter/flutter#108356) * 5d31b07 [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during terminate (flutter/flutter#108310) * c8b5d10 Roll Flutter Engine from 5dcaeae6561b to 89e117b89c63 (1 revision) (flutter/flutter#108367) * e3d08fb Hide the debug banner in the PopupMenuButton example (flutter/flutter#108324)

[flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during ter…

2153ec7

…minate Fixes an issue where the flutter_tester device may not be cleaned up correctly if we just terminate the Flutter process.

DanTup requested a review from christopherfujino July 25, 2022 18:15

flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 25, 2022

christopherfujino approved these changes Jul 25, 2022

View reviewed changes

Update integration test expectations

a24feb3

Revert accidental commit

94c88cf

DanTup merged commit 5d31b07 into flutter:master Jul 26, 2022

DanTup deleted the send-app-stop branch July 26, 2022 09:48

This was referenced Jul 26, 2022

Roll Flutter from fa39206a2a34 to e3d08fb6d325 (23 revisions) flutter/plugins#6147

Merged

Roll Flutter from bbec650b64bc to e3d08fb6d325 (197 revisions) flutter/packages#2383

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jul 26, 2022

5d31b07 [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach du…

1653171

…ring terminate (flutter/flutter#108310)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 26, 2022

5d31b07 [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach du…

9704e8b

…ring terminate (flutter/flutter#108310)

engine-flutter-autoroll mentioned this pull request Jul 26, 2022

Roll Flutter from bbec650b64bc to 2f551d30086a (198 revisions) flutter/packages#2384

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 26, 2022

5d31b07 [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach du…

5710bdd

…ring terminate (flutter/flutter#108310)

engine-flutter-autoroll mentioned this pull request Jul 26, 2022

Roll Flutter from bbec650b64bc to 527ee1d7286e (199 revisions) flutter/packages#2387

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 26, 2022

5d31b07 [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach du…

3fa8830

…ring terminate (flutter/flutter#108310)

engine-flutter-autoroll mentioned this pull request Jul 26, 2022

Roll Flutter from bbec650b64bc to 87938cec4426 (200 revisions) flutter/packages#2389

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 26, 2022

5d31b07 [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach du…

4f20598

…ring terminate (flutter/flutter#108310)

engine-flutter-autoroll mentioned this pull request Jul 26, 2022

Roll Flutter from bbec650b64bc to e05ae3cac799 (202 revisions) flutter/packages#2390

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 26, 2022

5d31b07 [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach du…

50a2703

…ring terminate (flutter/flutter#108310)

engine-flutter-autoroll mentioned this pull request Jul 26, 2022

Roll Flutter from bbec650b64bc to fe16c206ea19 (203 revisions) flutter/packages#2391

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 26, 2022

5d31b07 [flutter_tools] [dap] Ensure DAP sends app.stop/app.detach du…

44281db

…ring terminate (flutter/flutter#108310)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during terminate #108310

[flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during terminate #108310

Uh oh!

DanTup commented Jul 25, 2022

Uh oh!

christopherfujino left a comment

Uh oh!

christopherfujino commented Jul 25, 2022

Uh oh!

DanTup commented Jul 25, 2022

Uh oh!

christopherfujino commented Jul 25, 2022

Uh oh!

DanTup commented Jul 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during terminate #108310

[flutter_tools] [dap] Ensure DAP sends app.stop/app.detach during terminate #108310

Uh oh!

Conversation

DanTup commented Jul 25, 2022

Pre-launch Checklist

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

christopherfujino commented Jul 25, 2022

Uh oh!

DanTup commented Jul 25, 2022

Uh oh!

christopherfujino commented Jul 25, 2022

Uh oh!

DanTup commented Jul 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants