onsi / gomega

Ginkgo's Preferred Matcher Library
http://onsi.github.io/gomega/
MIT License
2.13k stars 282 forks source link

gleak will complain with leaked go-routines when using t.Parallel() #761

Closed advdv closed 1 month ago

advdv commented 1 month ago

I'm on Go version go version go1.22.3 darwin/arm64 and when I create a test like this:

package worker_test

import (
    "testing"

    . "github.com/onsi/ginkgo/v2"
    . "github.com/onsi/gomega"
    . "github.com/onsi/gomega/gleak"
)

func TestWorkerv2(t *testing.T) {
    t.Parallel() // removing this fixes it
    RegisterFailHandler(Fail)
    RunSpecs(t, "worker")
}

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

var _ = Describe("worker", func() {
    AfterEach(func() {
        Eventually(Goroutines).ShouldNot(HaveLeaked())
    })

    It("should foo", func() {})
    It("should bar", func() {})
})

And run ginkgo without any other arguments it will fail with:

Random Seed: 1717043403

Will run 2 of 2 specs
------------------------------
• [FAILED] [1.002 seconds]
worker [AfterEach] should foo
  [AfterEach] /Users/adam/Documents/Projects/github.com/crewlinker/atsback/worker/worker_test.go:22
  [It] /Users/adam/Documents/Projects/github.com/crewlinker/atsback/worker/worker_test.go:26

  [FAILED] Timed out after 1.001s.
  Expected not to leak 1 goroutines:
      goroutine 1 [chan receive]
          testing.tRunner.func1() at testing/testing.go:1650
          testing.tRunner(0x140004741a0, 0x1400023fc58) at testing/testing.go:1695
          testing.runTests(0x1400000ec00, {0x1011fddf0, 0x1, 0x1}, {0x1400023fd18?, 0x1004a1fd0?, 0x0?}) at testing/testing.go:2159
          testing.(*M).Run(0x140004475e0) at testing/testing.go:2027
          main.main() at _testmain.go:49
  In [AfterEach] at: /Users/adam/Documents/Projects/github.com/crewlinker/atsback/worker/worker_test.go:23 @ 05/30/24 06:30:06.071
------------------------------
• [FAILED] [1.002 seconds]
worker [AfterEach] should bar
  [AfterEach] /Users/adam/Documents/Projects/github.com/crewlinker/atsback/worker/worker_test.go:22
  [It] /Users/adam/Documents/Projects/github.com/crewlinker/atsback/worker/worker_test.go:27

  [FAILED] Timed out after 1.001s.
  Expected not to leak 1 goroutines:
      goroutine 1 [chan receive]
          testing.tRunner.func1() at testing/testing.go:1650
          testing.tRunner(0x140004741a0, 0x1400023fc58) at testing/testing.go:1695
          testing.runTests(0x1400000ec00, {0x1011fddf0, 0x1, 0x1}, {0x1400023fd18?, 0x1004a1fd0?, 0x0?}) at testing/testing.go:2159
          testing.(*M).Run(0x140004475e0) at testing/testing.go:2027
          main.main() at _testmain.go:49
  In [AfterEach] at: /Users/adam/Documents/Projects/github.com/crewlinker/atsback/worker/worker_test.go:23 @ 05/30/24 06:30:07.074
------------------------------

Summarizing 2 Failures:
  [FAIL] worker [AfterEach] should foo
  /Users/adam/Documents/Projects/github.com/crewlinker/atsback/worker/worker_test.go:23
  [FAIL] worker [AfterEach] should bar
  /Users/adam/Documents/Projects/github.com/crewlinker/atsback/worker/worker_test.go:23

Ran 2 of 2 Specs in 2.006 seconds
FAIL! -- 0 Passed | 2 Failed | 0 Pending | 0 Skipped
--- FAIL: TestWorkerv2 (2.01s)
FAIL

Ginkgo ran 1 suite in 3.861158792s

Test Suite Failed

It seems to be because of t.Parallel. Since this is a standard library function it might be reasonable to add this to the standard ignores that gleak has?

advdv commented 1 month ago

I see this case is explicitly mentioned in the docs: https://onsi.github.io/gomega/#well-known-non-leaky-goroutines

And since I'm not using I'll close this issue:

    BeforeEach(func() {
        goods := Goroutines()
        DeferCleanup(func() {
            Eventually(Goroutines).ShouldNot(HaveLeaked(goods))
        })
    })
blgm commented 1 month ago

Hi @advdv. I think it's worth adding that t.Parallel() won't have the effect of making Ginkgo tests run in parallel. Ginkgo runs tests in parallel by running separate processes started by the ginkgo command. The t.Parallel() setting allows Go func Test...(*testing.T) style tests to run in parallel using Goroutines. Why the difference? Ginkgo was written long before the t.Parallel() setting was added, and it's hard to retrofit Ginkgo to work using the same model as it would break many existing tests.

advdv commented 1 month ago

thank you for the response. honestly I'm just adding it because a linter is complaining about it. But I should probably just disable it for projects with a lot of Ginkgo.