CPIO files utility#523
Conversation
|
This PR introduces a call to I did not consider using a golang library (like this one) as I considered it was not worth adding an extra go dependency while having a fully supported |
Signed-off-by: David Cassany <dcassany@suse.com>
Signed-off-by: David Cassany <dcassany@suse.com>
atanasdinov
left a comment
atanasdinov
left a comment
There was a problem hiding this comment.
Great submission! I got a few comments and questions, but LGTM overall.
| return r.Run(command, args...) | ||
| } | ||
|
|
||
| func (r *Runner) RunContextWithPipe( |
There was a problem hiding this comment.
ReturnError/SideEffect values are being ignored. Do we want to add them as a proper unit test failure for mocked cpio errors?
|
|
||
| callErrChan := make(chan error, 1) | ||
| go func() { | ||
| _, e := io.Copy(stdout, pr) |
There was a problem hiding this comment.
I believe this can panic if stdout is not set. Is it an issue?
| Expect(r.RunContextWithPipe(context.Background(), callback, buff, nil, workDir, nil, "cat", "-", "secondLine")).To(Succeed()) | ||
| Expect(buff.String()).To(ContainSubstring("First line\nSecond line\n")) | ||
| }) | ||
| It("runs a command getting input from a pipe in a specific working directory", func() { |
There was a problem hiding this comment.
The test name is duplicated from above.
|
|
||
| if err = stdinPipeFn(stdinPipe); err != nil { | ||
| _ = stdinPipe.Close() | ||
| _ = cmd.Wait() |
There was a problem hiding this comment.
This call should contain the real error that we need to report. When failing, err = stdinPipeFn(stdinPipe) is likely to contain io: read/write on closed pipe because the subprocess would exit before consuming all of stdin.
One approach could be to keep both errors in the propagated error, e.g. return fmt.Errorf("callback returned an error: %w, pipe closed with: %w", cErr, err).
Have you ran into such issue?
2 participants
This PR extends the runner interface to support piped input and outputs and it also creates a small internal utility to create and extract CPIO files.