Skip to content

Commit

Permalink
fix false positive gleaks when using ginkgo -p (#577)
Browse files Browse the repository at this point in the history
  • Loading branch information
thediveo committed Aug 26, 2022
1 parent 40d7efe commit cb46517
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 23 deletions.
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

0 comments on commit cb46517

Please sign in to comment.