Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix false positive gleaks when using ginkgo -p #577

Merged
merged 2 commits into from Aug 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 21 additions & 5 deletions docs/index.md
Expand Up @@ -3056,11 +3056,27 @@ correctly terminate without triggering false positives. Please refer to the
interval (which defaults to 1s) and the polling interval (which defaults to
10ms).

This form of goroutine leak test can cause false positives in situations where a
test suite or dependency module uses additional goroutines. This simple form
only looks at all goroutines _after_ a test has run and filters out all
_well-known_ "non-leaky" goroutines, such as goroutines from Go's runtime and
the testing frameworks (such as Go's own testing package and Gomega).
Please note that this simplest form of goroutine leak test can cause false
positives in situations where a test suite or dependency module uses additional
goroutines. This simple form only looks at all goroutines _after_ a test has run
and filters out all _well-known_ "non-leaky" goroutines, such as goroutines from
Go's runtime and the testing frameworks (such as Go's own testing package and
Gomega).

### Ginkgo -p

In case you intend to run multiple package tests in parallel using `ginkgo -p
...`, you'll need to update any existing `BeforeSuite` or add new `BeforeSuite`s
in each of your packages. Calling `gleak.IgnoreGinkgoParallelClient` at the
beginning of `BeforeSuite` ensures that `gleak` updates its internal ignore list
to ignore a background goroutine related to the communication between Ginkgo and
the parallel packages under test.

```go
var _ = BeforeSuite(func() {
IgnoreGinkgoParallelClient()
})
```

### Using Goroutine Snapshots in Leak Testing

Expand Down
25 changes: 25 additions & 0 deletions gleak/ginkgo_parallel_client.go
@@ -0,0 +1,25 @@
package gleak

import "os"

// IgnoreGinkgoParallelClient must be called in a BeforeSuite whenever a test
// suite is run in parallel with other test suites using "ginkgo -p". Calling
// IgnoreGinkgoParallelClient checks for a Ginkgo-related background go routine
// and then updates gleak's internal ignore list to specifically ignore this
// background go routine by its ("random") ID.
func IgnoreGinkgoParallelClient() {
ignoreCreator := "net/rpc.NewClientWithCodec"
if os.Getenv("GINKGO_PARALLEL_PROTOCOL") == "HTTP" {
ignoreCreator = "net/http.(*Transport).dialConn"
}
ignores := []Goroutine{}
for _, g := range Goroutines() {
if g.CreatorFunction == ignoreCreator {
ignores = append(ignores, g)
}
}
if len(ignores) == 0 {
return
}
standardFilters = append(standardFilters, IgnoringGoroutines(ignores))
}
7 changes: 7 additions & 0 deletions gleak/gleak_suite_test.go
Expand Up @@ -7,6 +7,13 @@ import (
. "github.com/onsi/gomega"
)

// In case this suite is run in parallel with other test suites using "ginkgo
// -p", then there is a Ginkgo-related background go routine that we need to
// ignore in all tests and that can be identified only by its random ID.
var _ = BeforeSuite(func() {
IgnoreGinkgoParallelClient()
})

func TestGleak(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Gleak Suite")
Expand Down
38 changes: 20 additions & 18 deletions gleak/have_leaked_matcher.go
Expand Up @@ -21,11 +21,11 @@ import (
//
// That is, with ReportFilenameWithPath==false:
//
// foo/bar.go:123
// foo/bar.go:123
//
// Or with ReportFilenameWithPath==true:
//
// /home/goworld/coolprojects/mymodule/foo/bar.go:123
// /home/goworld/coolprojects/mymodule/foo/bar.go:123
var ReportFilenameWithPath = false

// standardFilters specifies the always automatically included no-leak goroutine
Expand All @@ -48,6 +48,8 @@ var standardFilters = []types.GomegaMatcher{
gomega.And(IgnoringTopFunction("runtime.goexit1"), IgnoringCreator("github.com/onsi/ginkgo/v2/internal.(*Suite).runNode")),
IgnoringTopFunction("github.com/onsi/ginkgo/v2/internal/interrupt_handler.(*InterruptHandler).registerForInterrupts..."),
IgnoringTopFunction("github.com/onsi/ginkgo/internal/specrunner.(*SpecRunner).registerForInterrupts"),
IgnoringCreator("github.com/onsi/ginkgo/v2/internal.(*genericOutputInterceptor).ResumeIntercepting"),
IgnoringCreator("github.com/onsi/ginkgo/v2/internal.(*genericOutputInterceptor).ResumeIntercepting..."),

// goroutines of Go's own testing package for its own workings...
IgnoringTopFunction("testing.RunTests [chan receive]"),
Expand Down Expand Up @@ -83,44 +85,44 @@ var standardFilters = []types.GomegaMatcher{
// Eventually's default timeout and polling interval settings, but these can be
// overridden as usual:
//
// // Remember to use "Goroutines" and not "Goroutines()" with Eventually()!
// Eventually(Goroutines).ShouldNot(HaveLeaked())
// Eventually(Goroutines).WithTimeout(5 * time.Second).ShouldNot(HaveLeaked())
// // Remember to use "Goroutines" and not "Goroutines()" with Eventually()!
// Eventually(Goroutines).ShouldNot(HaveLeaked())
// Eventually(Goroutines).WithTimeout(5 * time.Second).ShouldNot(HaveLeaked())
//
// In its simplest form, an expected non-leaky goroutine can be identified by
// passing the (fully qualified) name (in form of a string) of the topmost
// function in the backtrace. For instance:
//
// Eventually(Goroutines).ShouldNot(HaveLeaked("foo.bar"))
// Eventually(Goroutines).ShouldNot(HaveLeaked("foo.bar"))
//
// This is the shorthand equivalent to this explicit form:
//
// Eventually(Goroutines).ShouldNot(HaveLeaked(IgnoringTopFunction("foo.bar")))
// Eventually(Goroutines).ShouldNot(HaveLeaked(IgnoringTopFunction("foo.bar")))
//
// HaveLeak also accepts passing a slice of Goroutine objects to be considered
// non-leaky goroutines.
//
// snapshot := Goroutines()
// DoSomething()
// Eventually(Goroutines).ShouldNot(HaveLeaked(snapshot))
// snapshot := Goroutines()
// DoSomething()
// Eventually(Goroutines).ShouldNot(HaveLeaked(snapshot))
//
// Again, this is shorthand for the following explicit form:
//
// snapshot := Goroutines()
// DoSomething()
// Eventually(Goroutines).ShouldNot(HaveLeaked(IgnoringGoroutines(snapshot)))
// snapshot := Goroutines()
// DoSomething()
// Eventually(Goroutines).ShouldNot(HaveLeaked(IgnoringGoroutines(snapshot)))
//
// Finally, HaveLeaked accepts any GomegaMatcher and will repeatedly pass it a
// Goroutine object: if the matcher succeeds, the Goroutine object in question
// is considered to be non-leaked and thus filtered out. While the following
// built-in Goroutine filter matchers should hopefully cover most situations,
// any suitable GomegaMatcher can be used for tricky leaky Goroutine filtering.
//
// IgnoringTopFunction("foo.bar")
// IgnoringTopFunction("foo.bar...")
// IgnoringTopFunction("foo.bar [chan receive]")
// IgnoringGoroutines(expectedGoroutines)
// IgnoringInBacktrace("foo.bar.baz")
// IgnoringTopFunction("foo.bar")
// IgnoringTopFunction("foo.bar...")
// IgnoringTopFunction("foo.bar [chan receive]")
// IgnoringGoroutines(expectedGoroutines)
// IgnoringInBacktrace("foo.bar.baz")
func HaveLeaked(ignoring ...interface{}) types.GomegaMatcher {
m := &HaveLeakedMatcher{filters: standardFilters}
for _, ign := range ignoring {
Expand Down