[flutter_plugin_tools] Improve and test 'format' #4145
Conversation
- Adds unit tests, as there are currently none. - Adds more graceful failure handling. - Adds an internal ignore list to skip files that don't need to be formatted that showed up during local testing. - Adds a note explaining that it's intentially not using the new base command due to performance issues.
|
Originally this was going to be a PR to convert to PackageLoopingCommand, but that turned out to be terrible. There's more that could be done, like adding flags to control different languages (e.g., to allow someone to format Dart+Java without having clang-format installed), but the more explicit failure handling and the unit testing gives us a better foundation for making future improvements as follow-up later. |
| commandError = e; | ||
| }); | ||
|
|
||
| expect(commandError, isA<ToolExit>()); |
There was a problem hiding this comment.
Why not also expect that the tool is using retcode 5? (const int _exitJavaFormatFailed = 5;)
There was a problem hiding this comment.
In practice we never actually use the exit code, so we've never bothered with testing them; I figured we could add tests later if we ever decide to do something with them. I use them in the code because it's easy, and makes the line self-documenting.
| !path.contains( | ||
| pathFragmentForDirectories(<String>['example', 'build'])) && |
There was a problem hiding this comment.
Can these be implemented with some util from the path library? maybe isWithin? (probably not, it looks like it expects absolute paths)
There was a problem hiding this comment.
I looked a bit before writing this, and couldn't find anything that did arbitrary relate path containment.
- Adds unit tests, as there are currently none. - Adds more graceful failure handling. - Adds an internal ignore list to skip files that don't need to be formatted that showed up during local testing. - Adds a note explaining that it's intentially not using the new base command due to performance issues.
* upstream_master: (40 commits) [image_picker] Image picker fix camera device (flutter#3898) [flutter_plugin_tools] Improve license-check output (flutter#4154) [webview_flutter] Fix broken keyboard issue link (flutter#3266) [flutter_plugin_tools] Support format on Windows (flutter#4150) [flutter_plugin_tools] Make unit tests pass on Windows (flutter#4149) [image_picker_for_web] Migrate image_picker to package:cross_file (flutter#4083) [various] Prepare plugin repo for binding API improvements (flutter#4148) [quick_actions] Add const constructor (flutter#4131) [in_app_purchase] Add iOS currency symbol to ProductDetails (flutter#4144) [in_app_purchase] Added priceCurrencySymbol to SkuDetailsWrapper (flutter#4114) [image_picker_platform_interface] Add methods that return package:cross_file (flutter#4072) [flutter_plugin_tools] Improve and test 'format' (flutter#4145) [flutter_plugin_tools] Only check target packages in analyze (flutter#4146) [in_app_purchase] Fix crash when retrieveReceiptWithError gives an error. (flutter#4138) [video_player] Pause video when it completes (flutter#3727) [in_app_purchase] Add currencySymbol to ProductDetails (flutter#4115) [in_app_purchase] Add documentation for price change confirmations (flutter#4092) [camera] android-rework part 8: Supporting modules for final implementation (flutter#4054) [plugin_platform_interface] Fix README broken link (flutter#4143) [various] Prepare plugin repo for binding API improvements (flutter#4137) ...
- Adds unit tests, as there are currently none. - Adds more graceful failure handling. - Adds an internal ignore list to skip files that don't need to be formatted that showed up during local testing. - Adds a note explaining that it's intentially not using the new base command due to performance issues.
formatted that showed up during local testing.
command due to performance issues.
Pre-launch Checklist
dart format.)[shared_preferences]///).