Add logs to test command #233
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #233 +/- ##
==========================================
- Coverage 64.26% 63.65% -0.61%
==========================================
Files 75 75
Lines 1595 1640 +45
==========================================
+ Hits 1025 1044 +19
- Misses 493 513 +20
- Partials 77 83 +6
Continue to review full report at Codecov.
|
cmd/service/logs.go
Outdated
| for _, reader := range readers { | ||
| defer reader.Close() | ||
| go stdcopy.StdCopy(os.Stdout, os.Stderr, reader) | ||
| stdcopy.StdCopy(os.Stdout, os.Stderr, reader) |
There was a problem hiding this comment.
Remove the goroutine prevents to see the logs from many dependencies..
There was a problem hiding this comment.
Adding the goroutine, without having the <-utils.WaitForCancel() in the same function make the function executes the defer reader.Close().
We could return a function close that will call the reader.Close(). What do you think?
cmd/service/logs.go
Outdated
| for _, reader := range readers { | ||
| defer reader.Close() | ||
| go stdcopy.StdCopy(os.Stdout, os.Stderr, reader) | ||
| stdcopy.StdCopy(os.Stdout, os.Stderr, reader) |
There was a problem hiding this comment.
Adding the goroutine, without having the <-utils.WaitForCancel() in the same function make the function executes the defer reader.Close().
We could return a function close that will call the reader.Close(). What do you think?
cmd/service/test.go
Outdated
| @@ -38,6 +38,7 @@ func init() { | |||
| Test.Flags().StringP("event-filter", "e", "*", "Only log the data of the given event") | |||
| Test.Flags().StringP("task-filter", "r", "", "Only log the result of the given task") | |||
| Test.Flags().StringP("output-filter", "o", "", "Only log the data of the given output of a task result. If set, you also need to set the task in --task-filter") | |||
| Test.Flags().BoolP("full-logs", "f", false, "Display logs from service and its dependencies") | |||
There was a problem hiding this comment.
The flag full-logs is not explicit enough.
I suggest the name logs-dependencies and as explanation: "Also displays the logs of all service's dependencies".
Also please add this flag in the Example of this command.
There was a problem hiding this comment.
logs-dependencies is even more confusing I think because we don't know if we will have only the dependencies or all
Now the test command also have the logs from the service and the logs from all dependencies with the
--logs-allflag