feat: use very_good_test_runner to improve test output#308
Conversation
felangel
force-pushed
the
feat/test-output-format
branch
from
1c7b6d6 to
fb55ee0
Compare
|
my heart is going pitter patter |
| cmd: (cwd) async { | ||
| final installDone = progress?.call( | ||
| 'Running "flutter test" in $cwd', | ||
| void Function(String)? stdout, |
There was a problem hiding this comment.
Nit: Can we create a typedef for this?
|
|
||
| /// Run a command on directories with a `pubspec.yaml`. | ||
| Future<void> _runCommand<T>({ | ||
| required Future<T> Function(String cwd) cmd, |
There was a problem hiding this comment.
Nit: maybe have a type def for this?
There was a problem hiding this comment.
Why do you feel it's necessary? This is a private API and it's only used in this one spot. By providing a typedef developers would have to jump to the typedef to understand the function signature.
There was a problem hiding this comment.
Just for the sake of DRY, I've noticed this is repeated along the file.
Either way this is just a nit, I am fine either way
There was a problem hiding this comment.
Where else do you see it repeated? I only see Future<T> Function(String cwd) in one spot.
There was a problem hiding this comment.
Yeah, this isn't repeated, I guess I got into the flow because of the other comments
| if (processes.isEmpty) throw PubspecNotFound(); | ||
| Future<void> _flutterTest({ | ||
| String cwd = '.', | ||
| required void Function(String) stdout, |
There was a problem hiding this comment.
Nit: reuse the typedef here
There was a problem hiding this comment.
Same question here about whether you feel this is necessary? I'd prefer to leave it inline unless it's a public API (related to https://dart-lang.github.io/linter/lints/avoid_private_typedef_functions.html).
There was a problem hiding this comment.
Interesting lint rule, never thought about it that way, makes sense! Let's keep online, we can ignore all the similar comments
There was a problem hiding this comment.
The only reason I slightly prefer to leave it as is is because let's say we create a typedef like:
typedef ArgumentCallback<T> = void Function(T argument);Then the new signature will look like:
Future<void> _flutterTest({
String cwd = '.',
required ArgumentCallback<String>? stdout,
required ArgumentCallback<String>? stderr,
}) {...}I don't really see the benefit of the typedef in this case because as a developer, I have to look at what ArgumentCallback means above and, from a readability standpoint, I personally prefer seeing the type inline when it's simple like this especially if it's used in a private method.
Let me know what you think 👍
There was a problem hiding this comment.
Got it, I think it makes sense, I kind find the inline function type a little bit confusing on dart, hence why I prefer to use typedef, and as a bonus we DRY.
But I agree with you, being a private method, we have have freedom to leave like this, and it is easier to spot the require method contract. Lets keep like that 👍
| } | ||
| } | ||
|
|
||
| extension on int { |
3 participants
Description
testcommand #279)Type of Change