-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[flutter_plugin_tools] Migrate 'test' to new base command #4133
[flutter_plugin_tools] Migrate 'test' to new base command #4133
Conversation
This is probably the simplest command we have, and it hadn't been migrated, so that works out well as an intro to these refactors :) I think the only context you'll need here is to look at https://github.com/flutter/plugins/blob/master/script/tool/lib/src/common/package_looping_command.dart Note that this is divided into two commits: the first is just a simple local refactor to pull the bulk of the logic into helpers and do some minor cleanup on them (e.g., using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great refactor. The code is wwwway simpler to follow now!
// packages | ||
int exitCode = await processRunner.runAndStream( | ||
'dart', | ||
<String>['pub', 'get'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Isn't it slightly odd that flutter tests don't attempt to flutter pub get
too?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the comment just above this line.
|
||
print('RUNNING $packageName tests...'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot of missing "reporting". Is that done by the PackageLoopingCommand now? (not that I'm going to miss it, just making sure that the output log stays useful)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Standardized output for starting each package's run is part of the base command, yes.
Migrates `test` to the new package looping base command. Refactors the bulk of the logic to helpers for easier understanding of the flow. Part of flutter/flutter#83413
Migrates `test` to the new package looping base command. Refactors the bulk of the logic to helpers for easier understanding of the flow. Part of flutter/flutter#83413
Migrates
test
to the new package looping base command.Refactors the bulk of the logic to helpers for easier understanding of the flow.
Part of flutter/flutter#83413
Pre-launch Checklist
dart format
.)[shared_preferences]
///
).