Add basic reporting to the test runner #624
Conversation
github-actions
bot
added
the
package: patrol_cli
bartekpacia
force-pushed
the
feature/add_test_reporting
branch
from
fbc5f2b to
4cc8c64
Compare
bartekpacia
changed the title
TestRunner
bartekpacia
force-pushed
the
feature/add_test_reporting
branch
2 times, most recently
from
99b8868 to
4c0eccf
Compare
bartekpacia
force-pushed
the
feature/add_test_reporting
branch
from
4c0eccf to
0a8ab25
Compare
bartekpacia
force-pushed
the
feature/add_test_reporting
branch
from
087efff to
ef9bb0b
Compare
| late TestRunnerResult result; | ||
| unawaited(() async { | ||
| result = await testRunner.run(); | ||
| }()); |
There was a problem hiding this comment.
The other tests below use regular awaits to get the results – much better since you won't run into the risk of the log output not catching up with the intermediate delays
There was a problem hiding this comment.
I'm doing it this way because of https://github.com/dart-lang/fake_async/issues/51. If you know a solution to this ("How to await function inside of fakeAsync()), then I'd be happy if you shared it.
BTW the delays are entirely controlled by the fake async, so if the log output "won't catch up", then it's a bug and it's good that tests will fail in that case.
There was a problem hiding this comment.
Something like expectLater won't work the way you want?
There was a problem hiding this comment.
Unfortunately, it doesn't. See this commit where I tried this out.
There was a problem hiding this comment.
If you have an idea how to make this work, I'll be happy to hear it.
| targetRunResults.add( | ||
| TargetRunResult(target: target, device: device, runs: targetRuns), | ||
| ); |
There was a problem hiding this comment.
I know it's probably not that much of an issue here, but having TargetRunResult as an Equatable and then mutating its fields seems like a risky maneuver
There was a problem hiding this comment.
Why? I don't think it breaks Equatable's contract.
There was a problem hiding this comment.
I think it's ok. Besides the fact that i do not like mutability at all, but whether it is Equatable or not doesn't change anything here.
| targetRunResults.add( | ||
| TargetRunResult(target: target, device: device, runs: targetRuns), | ||
| ); |
There was a problem hiding this comment.
I think it's ok. Besides the fact that i do not like mutability at all, but whether it is Equatable or not doesn't change anything here.
| late TestRunnerResult result; | ||
| unawaited(() async { | ||
| result = await testRunner.run(); | ||
| }()); |
There was a problem hiding this comment.
Something like expectLater won't work the way you want?
Fixes #598
Also mostly fixes _ #561, but #561 is a bit broader, so I'm not closing #561