From a5eb7adfc79e707f6cccd931c64a572dee0a9737 Mon Sep 17 00:00:00 2001 From: bogdanteleaga Date: Fri, 1 Apr 2016 00:18:37 +0300 Subject: [PATCH] exec: always return at least an empty ExecResponse --- exec/exec.go | 16 ++++++++++++---- exec/exec_linux_test.go | 4 ---- exec/exec_test.go | 4 +--- exec/exec_windows_test.go | 3 --- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/exec/exec.go b/exec/exec.go index b66e7c985..a66281ec2 100644 --- a/exec/exec.go +++ b/exec/exec.go @@ -196,7 +196,12 @@ func (r *RunParams) Wait() (*ExecResponse, error) { } logger.Infof("run result: %v", ee) } - return result, err + + if err != nil { + return nil, errors.Trace(err) + } + + return result, nil } // ErrCancelled is returned by WaitWithCancel in case it successfully manages to kill @@ -232,7 +237,10 @@ func (r *RunParams) WaitWithCancel(cancel <-chan struct{}) (*ExecResponse, error select { case resWithError := <-done: - return resWithError.execResult, errors.Trace(resWithError.err) + if resWithError.err != nil { + return nil, errors.Trace(resWithError.err) + } + return resWithError.execResult, nil case <-cancel: logger.Debugf("attempting to kill process") err := r.KillProcess(r.ps.Process) @@ -243,8 +251,8 @@ func (r *RunParams) WaitWithCancel(cancel <-chan struct{}) (*ExecResponse, error // After we issue a kill we expect the wait above to return within timeWaitForKill. // In case it doesn't we just go on and assume the process is stuck, but we don't block select { - case resWithError := <-done: - return resWithError.execResult, ErrCancelled + case <-done: + return nil, ErrCancelled case <-_clock.After(timeWaitForKill): return nil, errors.Errorf("tried to kill process %v, but timed out", r.ps.Process.Pid) } diff --git a/exec/exec_linux_test.go b/exec/exec_linux_test.go index ad3ca12e9..60f2cbb11 100644 --- a/exec/exec_linux_test.go +++ b/exec/exec_linux_test.go @@ -10,10 +10,6 @@ import ( "github.com/juju/utils/exec" ) -// 0 is thrown by linux because RunParams.Wait -// only sets the code if the process exits cleanly -const cancelErrCode = 0 - func (*execSuite) TestRunCommands(c *gc.C) { newDir := c.MkDir() diff --git a/exec/exec_test.go b/exec/exec_test.go index ed79ada25..a077c7c53 100644 --- a/exec/exec_test.go +++ b/exec/exec_test.go @@ -38,9 +38,7 @@ func (*execSuite) TestWaitWithCancel(c *gc.C) { cancelChan <- struct{}{} result, err := params.WaitWithCancel(cancelChan) c.Assert(err, gc.Equals, exec.ErrCancelled) - c.Assert(string(result.Stdout), gc.Equals, "") - c.Assert(string(result.Stderr), gc.Equals, "") - c.Assert(result.Code, gc.Equals, cancelErrCode) + c.Assert(result, gc.IsNil) } func (s *execSuite) TestKillAbortedIfUnsuccessfull(c *gc.C) { diff --git a/exec/exec_windows_test.go b/exec/exec_windows_test.go index b4fd60d96..cc8e5e7a5 100644 --- a/exec/exec_windows_test.go +++ b/exec/exec_windows_test.go @@ -14,9 +14,6 @@ import ( "github.com/juju/utils/exec" ) -// 1 is thrown by powershell after the a command is cancelled -const cancelErrCode = 1 - // longPath is copied over from the symlink package. This should be removed // if we add it to gc or in some other convenience package func longPath(path string) ([]uint16, error) {