Skip to content

Commit

Permalink
internal/testenv: always Cleanup to appease go vet
Browse files Browse the repository at this point in the history
vet from Go 1.22 complains about cancelCtx being potentially leaked:

    exec.go:140:4: the cancelCtx function is not used on all paths (possible context leak)
    exec.go:192:2: this return statement may be reached without using the cancelCtx var defined on line 140

The code was fine with that, and in practice the leak does not happen,
but having the vet warning around is still distracting.
Thankfully, Go 1.14 is very old at this point, and per go.mod,
this module now requires Go 1.18 or later, so we can clean this up.

Change-Id: I4c402ad233e9b33a6d51c98ba0833c67fe01b209
Reviewed-on: https://go-review.googlesource.com/c/tools/+/563675
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
mvdan committed Feb 13, 2024
1 parent afd8428 commit 1b39a8b
Showing 1 changed file with 9 additions and 17 deletions.
26 changes: 9 additions & 17 deletions internal/testenv/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ func NeedsExec(t testing.TB) {
// for an arbitrary grace period before the test's deadline expires,
// - if Cmd has the Cancel field, fails the test if the command is canceled
// due to the test's deadline, and
// - if supported, sets a Cleanup function that verifies that the test did not
// leak a subprocess.
// - sets a Cleanup function that verifies that the test did not leak a subprocess.
func CommandContext(t testing.TB, ctx context.Context, name string, args ...string) *exec.Cmd {
t.Helper()
NeedsExec(t)
Expand Down Expand Up @@ -173,21 +172,14 @@ func CommandContext(t testing.TB, ctx context.Context, name string, args ...stri
rWaitDelay.Set(reflect.ValueOf(gracePeriod))
}

// t.Cleanup was added in Go 1.14; for earlier Go versions,
// we just let the Context leak.
type Cleanupper interface {
Cleanup(func())
}
if ct, ok := t.(Cleanupper); ok {
ct.Cleanup(func() {
if cancelCtx != nil {
cancelCtx()
}
if cmd.Process != nil && cmd.ProcessState == nil {
t.Errorf("command was started, but test did not wait for it to complete: %v", cmd)
}
})
}
t.Cleanup(func() {
if cancelCtx != nil {
cancelCtx()
}
if cmd.Process != nil && cmd.ProcessState == nil {
t.Errorf("command was started, but test did not wait for it to complete: %v", cmd)
}
})

return cmd
}
Expand Down

0 comments on commit 1b39a8b

Please sign in to comment.