Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TestRunner could be improved to report more results, better #561

Closed
bartekpacia opened this issue Nov 3, 2022 · 10 comments
Closed

TestRunner could be improved to report more results, better #561

bartekpacia opened this issue Nov 3, 2022 · 10 comments
Labels
feature New feature request P1 High-priority issues at the top of the work list package: patrol_cli Related to the patrol_cli package

Comments

@bartekpacia
Copy link
Contributor

bartekpacia commented Nov 3, 2022

We could print a nice test result summary, similar to what spec does:

Screenshot 2022-11-03 at 6 32 54 PM

Without colors it looks like this:

PASS  test/command_runner_test.dart
PASS  test/features/drive/test_runner_test.dart
PASS  test/features/drive/drive_command_test.dart
PASS  test/features/drive/device_finder_test.dart
PASS  test/features/drive/test_finder_test.dart
PASS  test/common/artifacts_repository_test.dart

For Patrol, I'd like to print something like this:

PASS  integration_test/core_cutter_test.dart.dart (build passed, 10/10 runs passed)
FAIL  integration_test/dual_cbr_test.dart  (build failed)
FAIL  integration_test/hsv_test.dart (build passed, 9/10 runs passed)
CANC  integration_test/srd_test.dart  (build passed, 3/10 runs passed)

To learn more about failures, see the logs above. grep is your friend.
Code already written Some code that would go in #560 but I decided it doesn't fit there.
// TODO: Use this
class SingleTargetRunResult {
  SingleTargetRunResult({
    required this.target,
    required this.device,
    required this.plannedRuns,
    this.buildPassed = false,
    required this.runs,
  });

  final String target;
  final Device device;
  bool buildPassed;

  final int plannedRuns;
  final List<RunStatus> runs;
}

And then TestRunner.run()'s signature would be:

Future<List<SingleTargetRunResult>> run() {}
  • This would make code in DriveCommand cleaner (it'd be moved to TestRunner)
  • All the complexity would be contained inside TestRunner and properly tested

To be precise, this issue is only about nicer logs in the CLI, not about detailed error reporting using flutter test with the JSON reporter to convert test logs into a JUnit format. See #421 and #495 for this.

cc @jBorkowska @shilangyu @jakubfijalkowski – as always, if you have any thought about this, share them here :)

@jBorkowska
Copy link
Collaborator

Is this a place to add the error messages for why a test failed? Or would it be kept separate?

@bartekpacia
Copy link
Contributor Author

bartekpacia commented Nov 7, 2022

No. To find you why a test failed, you'd have to still look in the logs.

I mean, I don't know how we could make it fit in 1 line and make it look good.

@jBorkowska
Copy link
Collaborator

From my perspective: I need a place, where I can easily find info about which test failed and why. Logs are quite hard to read (at least now), that's why I'm asking.

@jakubfijalkowski
Copy link
Member

jakubfijalkowski commented Nov 7, 2022

Jest prints, apart from the passed/failed marker, a summary of failed tests at the end of the output. This is, IMO, quite good middle-ground between being fancy (so a pass/fail colored marker) and being useful. If everything passes you have clean output, if something fails, you see what failed right there, without looking for it.

@bartekpacia
Copy link
Contributor Author

bartekpacia commented Nov 7, 2022

@jakubfijalkowski Jest is great, and its equivalent in Dart is spec (in this context I mean spec_cli, the interactive tester runner) . I use it often and it just works and improves test readability.

Maybe we should look for ways to integrate with them, or borrow their interactive test runner code :)

@bartekpacia
Copy link
Contributor Author

Cross-referencing #528

@bartekpacia bartekpacia added feature New feature request package: patrol_cli Related to the patrol_cli package labels Nov 17, 2022
@bartekpacia bartekpacia self-assigned this Nov 18, 2022
@neiljaywarner
Copy link

@bartekpacia yes, something like this would be awesome, our QE says patrol is awesome but hard to find the issues in the log etc.

@bartekpacia
Copy link
Contributor Author

Yeah, we're aware of it. We hope to improve this!

For now, you can use flutter logs while running patrol test to view Dart-side logs.

@bartekpacia
Copy link
Contributor Author

This is a possible duplicate of #870.

@bartekpacia bartekpacia removed their assignment Nov 20, 2023
@jBorkowska jBorkowska added P1 High-priority issues at the top of the work list P2 Issues not at the top of the work list and removed P1 High-priority issues at the top of the work list P2 Issues not at the top of the work list labels Nov 20, 2023
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar problem, please file a new issue. Make sure to follow the template and provide all the information necessary to reproduce the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New feature request P1 High-priority issues at the top of the work list package: patrol_cli Related to the patrol_cli package
Projects
None yet
Development

No branches or pull requests

4 participants