give `throwsToolExit` a more useful description by andrewkolos · Pull Request #136694 · flutter/flutter

andrewkolos · 2023-10-17T01:56:39Z

Alters how throwToolExit creates its matcher. This results is an improved description of the matcher.

The mismatch description isn't improved by this, but I writing an entirely custom matcher to fix this isn't ideal either. We can instead mitigate the issue by augmenting the toString implementation of ToolExit to include the exit code, if it is non-null.

With these changes, the first few lines of output from a test would look like this:

Expected: throws <Instance of 'ToolExit'> with `exitCode`: <42> and `message`: contains 'message'
  Actual: <Closure: () => Never>
   Which: threw ToolExit:<Exit code: 41232. Error: message>

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.

christopherfujino · 2023-10-19T21:12:31Z

packages/flutter_tools/lib/src/base/common.dart

Is this really helpful to the user?

My concern is more that it's a little messier to have: Exit code: 1. Error: ...

Yeah, you ask a fair question. We already print the exit code later in the typical tool exit workflow, so this could result in redundant noise for end-users. It would be nice if we could include it in the matcher's mismatch output; but, as far as I can tell, this would involve writing a bunch of custom matcher code, which I don't want to do. Maybe I will the next time I have troubles with throwsToolExit :)

I've reverted this part of the change (I did keep the "Exception" to "Error" change)

christopherfujino

question about including the exit code, but otherwise LGTM

…ter#136694)

Roll Flutter from c555599 to 5907c97 (45 revisions) flutter/flutter@c555599...5907c97 2023-10-27 [email protected] Add `isLogicalKeyPressed` to `KeyEvent` (flutter/flutter#136856) 2023-10-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Ensure `flutter build apk --release` optimizes+shrinks platform code" (flutter/flutter#137433) 2023-10-27 [email protected] Roll Flutter Engine from bedc49efc85c to a198ad4e740d (20 revisions) (flutter/flutter#137429) 2023-10-27 [email protected] Make `SemanticsNode.isMergedIntoParent` Readonly (flutter/flutter#137304) 2023-10-27 [email protected] Manual roll Flutter Engine from bedc49efc85c to 71e1a0430232 (16 revisions) (flutter/flutter#137422) 2023-10-27 [email protected] Roll Packages from fea24c5 to 2af6954 (5 revisions) (flutter/flutter#137421) 2023-10-27 [email protected] Run tests on either macOS 12 or 13 (flutter/flutter#137365) 2023-10-27 [email protected] Revert "Update `DataTable` test when data row is pressed for Material 3 (#137230)" (flutter/flutter#137407) 2023-10-27 [email protected] Revert "Reland - Update `OutlinedButton` tests for Material 3 (#136809) (#137247)" (flutter/flutter#137406) 2023-10-27 [email protected] Ensure `flutter build apk --release` optimizes+shrinks platform code (flutter/flutter#136880) 2023-10-27 [email protected] Update `DataTable` test when data row is pressed for Material 3 (flutter/flutter#137230) 2023-10-27 [email protected] Reland - Update `OutlinedButton` tests for Material 3 (#136809) (flutter/flutter#137247) 2023-10-27 [email protected] update asset manifest file name referenced in `WebServiceWorker` (flutter/flutter#135954) 2023-10-27 [email protected] give `throwsToolExit` a more useful description (flutter/flutter#136694) 2023-10-27 [email protected] Add ConstrainedLayoutBuilder.updateShouldRebuild() (flutter/flutter#136691) 2023-10-27 [email protected] Fix. typos (flutter/flutter#137325) 2023-10-27 [email protected] Roll Flutter Engine from 87f384c2d70b to bedc49efc85c (1 revision) (flutter/flutter#137382) 2023-10-26 [email protected] Fix Typos (flutter/flutter#137292) 2023-10-26 [email protected] AnimationController should dispatch creation in constructor. (flutter/flutter#134839) 2023-10-26 [email protected] Roll Flutter Engine from 9788bb9ff83e to 87f384c2d70b (4 revisions) (flutter/flutter#137380) 2023-10-26 [email protected] Roll Flutter Engine from ce1c1ee54107 to 9788bb9ff83e (3 revisions) (flutter/flutter#137373) 2023-10-26 [email protected] Marks Windows_arm64 windows_startup_test to be unflaky (flutter/flutter#137228) 2023-10-26 [email protected] Marks Windows_arm64 flutter_tool_startup__windows to be unflaky (flutter/flutter#137229) 2023-10-26 [email protected] Marks Windows_arm64 windows_home_scroll_perf__timeline_summary to be unflaky (flutter/flutter#137221) 2023-10-26 [email protected] Declare dependency on copyFlutterAssetsTask in bundleAarTask (flutter/flutter#137370) 2023-10-26 [email protected] Marks Windows_arm64 complex_layout_win_desktop__start_up to be unflaky (flutter/flutter#137225) 2023-10-26 [email protected] Roll Flutter Engine from 394744d2c4d0 to ce1c1ee54107 (2 revisions) (flutter/flutter#137367) 2023-10-26 [email protected] Marks Windows_arm64 hot_mode_dev_cycle_win_target__benchmark to be unflaky (flutter/flutter#137217) 2023-10-26 [email protected] Marks Windows_arm64 flutter_view_win_desktop__start_up to be unflaky (flutter/flutter#137226) 2023-10-26 [email protected] Marks Windows_arm64 platform_view_win_desktop__start_up to be unflaky (flutter/flutter#137227) 2023-10-26 [email protected] Marks Windows_arm64 flutter_gallery_win_desktop__start_up to be unflaky (flutter/flutter#137224) 2023-10-26 [email protected] Marks Windows_arm64 hello_world_win_desktop__compile to be unflaky (flutter/flutter#137222) 2023-10-26 [email protected] Marks Windows_arm64 flutter_gallery_win_desktop__compile to be unflaky (flutter/flutter#137223) 2023-10-26 [email protected] Marks Windows_arm64 run_release_test_windows to be unflaky (flutter/flutter#137220) 2023-10-26 [email protected] Fix dislocated doc and comment on ThemeData localize cache (flutter/flutter#137315) 2023-10-26 [email protected] Marks Windows_arm64 platform_channel_sample_test_windows to be unflaky (flutter/flutter#137218) 2023-10-26 [email protected] Roll Flutter Engine from 7c5c8f587992 to 394744d2c4d0 (1 revision) (flutter/flutter#137364) 2023-10-26 [email protected] Roll Flutter Engine from 9363fe6ba503 to 7c5c8f587992 (3 revisions) (flutter/flutter#137363) 2023-10-26 [email protected] Roll Flutter Engine from 542f8bc4c019 to 9363fe6ba503 (2 revisions) (flutter/flutter#137354) 2023-10-26 [email protected] Unified analytics events for doctor validators (flutter/flutter#136647) 2023-10-26 [email protected] Roll Flutter Engine from d8132d5070bf to 542f8bc4c019 (2 revisions) (flutter/flutter#137349) 2023-10-26 [email protected] Marks Windows_arm64 run_debug_test_windows to be unflaky (flutter/flutter#137219) 2023-10-26 [email protected] Roll Flutter Engine from 0a6253dbfafd to d8132d5070bf (1 revision) (flutter/flutter#137347) 2023-10-26 [email protected] Roll Flutter Engine from 5da115661f01 to 0a6253dbfafd (1 revision) (flutter/flutter#137341) 2023-10-26 [email protected] Roll Flutter Engine from 61ae5ef94e9c to 5da115661f01 (1 revision) (flutter/flutter#137340) ...

…ter#136694)

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 17, 2023

andrewkolos changed the title ~~have throwsToolExit provide more useful mismatch descriptions~~ have throwsToolExit provide more useful descriptions Oct 17, 2023

andrewkolos marked this pull request as ready for review October 17, 2023 19:32

andrewkolos changed the title ~~have throwsToolExit provide more useful descriptions~~ give throwsToolExit a more useful description Oct 17, 2023

christopherfujino reviewed Oct 19, 2023

View reviewed changes

christopherfujino approved these changes Oct 19, 2023

View reviewed changes

andrewkolos added 5 commits October 26, 2023 14:03

improve throwsToolExit

ca9b8f7

experiment: add error code to ToolExit toString

ae2c224

close Hixie's TODO

1c7cf11

fix tests

c3953ea

revert adding exit code to ToolExit toString

f9e29da

andrewkolos force-pushed the upgrade-throws-tool-exit-matcher branch from f5d6a95 to f9e29da Compare October 26, 2023 21:03

andrewkolos added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 27, 2023

auto-submit bot merged commit 1328997 into flutter:master Oct 27, 2023

engine-flutter-autoroll mentioned this pull request Oct 27, 2023

Roll Flutter from c555599fbb4a to eba149fcc3dc (38 revisions) flutter/packages#5244

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 27, 2023

1328997 give throwsToolExit a more useful description (flutter/flut…

8bd8194

…ter#136694)

engine-flutter-autoroll mentioned this pull request Oct 27, 2023

Roll Flutter from c555599fbb4a to e75f85bffd46 (39 revisions) flutter/packages#5246

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 27, 2023

1328997 give throwsToolExit a more useful description (flutter/flut…

1682a2d

…ter#136694)

engine-flutter-autoroll mentioned this pull request Oct 27, 2023

Roll Flutter from c555599fbb4a to 8cfa9c478c76 (40 revisions) flutter/packages#5247

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 27, 2023

1328997 give throwsToolExit a more useful description (flutter/flut…

015ab3a

…ter#136694)

engine-flutter-autoroll mentioned this pull request Oct 27, 2023

Roll Flutter from c555599fbb4a to 3ef84471c188 (41 revisions) flutter/packages#5249

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 27, 2023

1328997 give throwsToolExit a more useful description (flutter/flut…

9df1b2c

…ter#136694)

engine-flutter-autoroll mentioned this pull request Oct 27, 2023

Roll Flutter from c555599fbb4a to 4cae1af41d13 (44 revisions) flutter/packages#5251

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 27, 2023

1328997 give throwsToolExit a more useful description (flutter/flut…

dca6811

…ter#136694)

engine-flutter-autoroll mentioned this pull request Oct 27, 2023

Roll Flutter from c555599fbb4a to 5907c9757e3f (45 revisions) flutter/packages#5252

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 27, 2023

1328997 give throwsToolExit a more useful description (flutter/flut…

824a682

…ter#136694)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024

1328997 give throwsToolExit a more useful description (flutter/flut…

3f2ff3c

…ter#136694)

andrewkolos deleted the upgrade-throws-tool-exit-matcher branch April 29, 2024 18:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

give `throwsToolExit` a more useful description#136694

give `throwsToolExit` a more useful description#136694
auto-submit[bot] merged 5 commits intoflutter:masterfrom
andrewkolos:upgrade-throws-tool-exit-matcher

andrewkolos commented Oct 17, 2023 •

edited

Loading

Uh oh!

christopherfujino Oct 19, 2023

Uh oh!

christopherfujino Oct 19, 2023

Uh oh!

andrewkolos Oct 19, 2023

Uh oh!

christopherfujino left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

andrewkolos commented Oct 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

christopherfujino Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

christopherfujino Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

andrewkolos Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

andrewkolos commented Oct 17, 2023 •

edited

Loading