Skip to content

Commit

Permalink
Merge pull request #3 from edwarnicke/ctxfix
Browse files Browse the repository at this point in the history
Fix bug in Start ctx handling
  • Loading branch information
edwarnicke authored May 19, 2020
2 parents a22a20b + 8218f0e commit 6176af8
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 4 deletions.
4 changes: 3 additions & 1 deletion exechelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ func Start(cmdStr string, options ...*Option) <-chan error {
// Set the context
var ctx context.Context
for _, option := range options {
ctx = option.Context
if option.Context != nil {
ctx = option.Context
}
}

// Construct the command args
Expand Down
14 changes: 11 additions & 3 deletions exechelper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"path"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"

Expand Down Expand Up @@ -60,10 +61,17 @@ func TestCombinedOutputWithDir(t *testing.T) {

func TestStartWithContext(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
errCh := exechelper.Start("sleep 600", exechelper.WithContext(ctx))
errCh := exechelper.Start("sleep 600", exechelper.WithContext(ctx), exechelper.CmdOption(func(cmd *exec.Cmd) error {
return nil
}))
cancel()
assert.IsType(t, &exec.ExitError{}, <-errCh) // Because we canceled we will get an exec.ExitError{}
assert.Empty(t, errCh)
select {
case err := <-errCh:
assert.IsType(t, &exec.ExitError{}, err) // Because we canceled we will get an exec.ExitError{}
assert.Empty(t, errCh)
case <-time.After(time.Second):
assert.Fail(t, "Failed to cancel context")
}
}

func TestCmdOptionErr(t *testing.T) {
Expand Down

0 comments on commit 6176af8

Please sign in to comment.