Skip to content

Commit 2be09d1

Browse files
committed
fix too many return statement code smell
1 parent f7e0f64 commit 2be09d1

File tree

2 files changed

+14
-13
lines changed

2 files changed

+14
-13
lines changed

command.go

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -194,10 +194,10 @@ func (c *Command) ExecuteContext(ctx context.Context) error {
194194
cmd.Stderr = c.StderrWriter
195195
cmd.Dir = c.WorkingDir
196196

197-
// Create timer only if timeout was set > 0 and context does
198-
// not have a deadline
197+
// Respect legacy timer setting only if timeout was set > 0
198+
// and context does not have a deadline
199199
_, hasDeadline := ctx.Deadline()
200-
if c.Timeout != 0 && !hasDeadline {
200+
if c.Timeout > 0 && !hasDeadline {
201201
subCtx, cancel := context.WithTimeout(ctx, c.Timeout)
202202
defer cancel()
203203
ctx = subCtx
@@ -209,24 +209,23 @@ func (c *Command) ExecuteContext(ctx context.Context) error {
209209
}
210210

211211
done := make(chan error, 1)
212-
go func() {
213-
done <- cmd.Wait()
214-
}()
212+
go func() { done <- cmd.Wait() }()
215213

216214
select {
217215
case <-ctx.Done():
218216
if err := cmd.Process.Kill(); err != nil {
219217
return fmt.Errorf("Timeout occurred and can not kill process with pid %v", cmd.Process.Pid)
220218
}
221-
if c.Timeout != 0 && !hasDeadline {
222-
return fmt.Errorf("Command timed out after %v", c.Timeout)
219+
220+
err := ctx.Err()
221+
if c.Timeout > 0 && !hasDeadline {
222+
err = fmt.Errorf("Command timed out after %v", c.Timeout)
223223
}
224-
return ctx.Err()
224+
return err
225225
case err := <-done:
226-
if err != nil {
227-
c.getExitCode(err)
228-
}
226+
c.getExitCode(err)
229227
}
228+
230229
c.executed = true
231230
return nil
232231
}

command_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,14 @@ func TestCommand_SetOptions(t *testing.T) {
133133
}
134134

135135
func TestCommand_WithContext(t *testing.T) {
136-
// Ensure context is favored over WithTimeout
136+
// ensure legacy timeout is honored
137137
cmd := NewCommand("sleep 3;", WithTimeout(1*time.Second))
138138
err := cmd.Execute()
139139
assert.NotNil(t, err)
140140
assert.Equal(t, "Command timed out after 1s", err.Error())
141141

142+
// set context timeout to 2 seconds to ensure
143+
// context takes precedence over timeout
142144
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
143145
defer cancel()
144146
err = cmd.ExecuteContext(ctx)

0 commit comments

Comments
 (0)