-
Notifications
You must be signed in to change notification settings - Fork 55
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
Return empty array when using --json flag for list commands #736
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #736 +/- ##
=======================================
Coverage 70.60% 70.61%
=======================================
Files 87 87
Lines 11197 11198 +1
=======================================
+ Hits 7906 7907 +1
Misses 2778 2778
Partials 513 513
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
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.
Should we add this assert to all other existing list
integration tests?
Except for |
(by assert I mean the "contain" on "stdout"). |
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.
Change makes sense and solves a very real pain point. Satisfied with the coverage provided by unit test + example integration test.
@Widcket we added unit tests for the renderer func + used it in 1 integration test. We felt it was enough for now :) |
It's not about the coverage, it's about the other integration tests that did not have an assert on the output. |
Of course, however you prefer. |
This PR finally makes it possible to assert on the output of empty lists. |
And there being a single function doing the output is an implementation detail, which is not relevant from the perspective of black-box integration tests. |
@Widcket completely agreed, we can follow up in another PR to have those covered as well. We're mostly just trying to use our time wisely at the moment for the upcoming release. :) |
Of course, prioritize as you must. |
🔧 Changes
When returning the output of a list command in combination with the --json flag, if there are no elements to display we should still return an empty array.
📚 References
🔬 Testing
📝 Checklist