diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-10-06 10:54:37 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-10-07 08:37:22 +0300 |
commit | 58f78e3adedb82b3216e1251574aa45bc4702940 (patch) | |
tree | 163857c5a7f9c5801afea634ca436b70a1c3e066 | |
parent | d8b43e951b747acc49a8d1fff1d59be9867a5eca (diff) |
testhelper: Move process leak checks into separate package
The function `MustHaveNoChildProcess()` is not a general test helper
that should be used by tests as they see fit, but is instead intended to
be run at the end of tests only. Move it into a separate package and
make it internal to the package to make it less discoverable by callers.
-rw-r--r-- | internal/testhelper/configure.go | 2 | ||||
-rw-r--r-- | internal/testhelper/leakage.go | 63 | ||||
-rw-r--r-- | internal/testhelper/testhelper.go | 67 |
3 files changed, 64 insertions, 68 deletions
diff --git a/internal/testhelper/configure.go b/internal/testhelper/configure.go index 521cb428b..071f16f56 100644 --- a/internal/testhelper/configure.go +++ b/internal/testhelper/configure.go @@ -46,7 +46,7 @@ func Run(m *testing.M, opts ...RunOption) { opt(&cfg) } - defer MustHaveNoChildProcess() + defer mustHaveNoChildProcess() cleanup, err := configure() if err != nil { diff --git a/internal/testhelper/leakage.go b/internal/testhelper/leakage.go new file mode 100644 index 000000000..f5f56c5f5 --- /dev/null +++ b/internal/testhelper/leakage.go @@ -0,0 +1,63 @@ +package testhelper + +import ( + "fmt" + "os" + "os/exec" + "strings" + "syscall" + "time" + + "gitlab.com/gitlab-org/gitaly/v14/internal/command" + "gitlab.com/gitlab-org/gitaly/v14/internal/helper/text" +) + +// mustHaveNoChildProcess panics if it finds a running or finished child +// process. It waits for 2 seconds for processes to be cleaned up by other +// goroutines. +func mustHaveNoChildProcess() { + waitDone := make(chan struct{}) + go func() { + command.WaitAllDone() + close(waitDone) + }() + + select { + case <-waitDone: + case <-time.After(2 * time.Second): + } + + mustFindNoFinishedChildProcess() + mustFindNoRunningChildProcess() +} + +func mustFindNoFinishedChildProcess() { + // Wait4(pid int, wstatus *WaitStatus, options int, rusage *Rusage) (wpid int, err error) + // + // We use pid -1 to wait for any child. We don't care about wstatus or + // rusage. Use WNOHANG to return immediately if there is no child waiting + // to be reaped. + wpid, err := syscall.Wait4(-1, nil, syscall.WNOHANG, nil) + if err == nil && wpid > 0 { + panic(fmt.Errorf("wait4 found child process %d", wpid)) + } +} + +func mustFindNoRunningChildProcess() { + pgrep := exec.Command("pgrep", "-P", fmt.Sprintf("%d", os.Getpid())) + desc := fmt.Sprintf("%q", strings.Join(pgrep.Args, " ")) + + out, err := pgrep.Output() + if err == nil { + pidsComma := strings.Replace(text.ChompBytes(out), "\n", ",", -1) + psOut, _ := exec.Command("ps", "-o", "pid,args", "-p", pidsComma).Output() + panic(fmt.Errorf("found running child processes %s:\n%s", pidsComma, psOut)) + } + + if status, ok := command.ExitStatus(err); ok && status == 1 { + // Exit status 1 means no processes were found + return + } + + panic(fmt.Errorf("%s: %w", desc, err)) +} diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index 30bd253ac..7a52ce3ee 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -16,8 +16,6 @@ import ( "os" "os/exec" "path/filepath" - "strings" - "syscall" "testing" "time" @@ -25,12 +23,9 @@ import ( log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitaly/v14/internal/command" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v14/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v14/internal/metadata/featureflag" - "go.uber.org/goleak" "google.golang.org/grpc/metadata" ) @@ -165,68 +160,6 @@ func GetLocalhostListener(t testing.TB) (net.Listener, string) { return l, addr } -// MustHaveNoGoroutines panics if it finds any Goroutines running. -func MustHaveNoGoroutines() { - if err := goleak.Find( - // opencensus has a "defaultWorker" which is started by the package's - // `init()` function. There is no way to stop this worker, so it will leak - // whenever we import the package. - goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start"), - ); err != nil { - panic(fmt.Errorf("goroutines running: %w", err)) - } -} - -// MustHaveNoChildProcess panics if it finds a running or finished child -// process. It waits for 2 seconds for processes to be cleaned up by other -// goroutines. -func MustHaveNoChildProcess() { - waitDone := make(chan struct{}) - go func() { - command.WaitAllDone() - close(waitDone) - }() - - select { - case <-waitDone: - case <-time.After(2 * time.Second): - } - - mustFindNoFinishedChildProcess() - mustFindNoRunningChildProcess() -} - -func mustFindNoFinishedChildProcess() { - // Wait4(pid int, wstatus *WaitStatus, options int, rusage *Rusage) (wpid int, err error) - // - // We use pid -1 to wait for any child. We don't care about wstatus or - // rusage. Use WNOHANG to return immediately if there is no child waiting - // to be reaped. - wpid, err := syscall.Wait4(-1, nil, syscall.WNOHANG, nil) - if err == nil && wpid > 0 { - panic(fmt.Errorf("wait4 found child process %d", wpid)) - } -} - -func mustFindNoRunningChildProcess() { - pgrep := exec.Command("pgrep", "-P", fmt.Sprintf("%d", os.Getpid())) - desc := fmt.Sprintf("%q", strings.Join(pgrep.Args, " ")) - - out, err := pgrep.Output() - if err == nil { - pidsComma := strings.Replace(text.ChompBytes(out), "\n", ",", -1) - psOut, _ := exec.Command("ps", "-o", "pid,args", "-p", pidsComma).Output() - panic(fmt.Errorf("found running child processes %s:\n%s", pidsComma, psOut)) - } - - if status, ok := command.ExitStatus(err); ok && status == 1 { - // Exit status 1 means no processes were found - return - } - - panic(fmt.Errorf("%s: %w", desc, err)) -} - // ContextOpt returns a new context instance with the new additions to it. type ContextOpt func(context.Context) (context.Context, func()) |