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

Support t.Parallel? #16

Closed
zchee opened this issue Jan 9, 2018 · 7 comments
Closed

Support t.Parallel? #16

zchee opened this issue Jan 9, 2018 · 7 comments

Comments

@zchee
Copy link
Contributor

zchee commented Jan 9, 2018

Hi, thanks for providing great package :)

BTW, I saw goleak internal source and found below switch...case:

case "testing.RunTests", "testing.(*T).Run":

But, I used t.Parallel func on my project testing. So, goleak suggested:

found unexpected goroutines

Is it the expected behavior? Or, Should I adding goleak.IgnoreTopFunction("testing.(*T).Parallel") to defer goleak.VerifyNoLeaks(t)? (and, is it correct use case?)

@houqp
Copy link

houqp commented Jun 4, 2018

Got hit by the same issue, is there any reason not to add t.Parallel to the whitelist?

@zchee
Copy link
Contributor Author

zchee commented Jun 4, 2018

/cc @prashantv

@prashantv
Copy link
Collaborator

I think adding testing.(*T).Parallel is reasonable for ignoring tests that are queued to be run later.

I don't think we can support goroutine verification in parallel tests though, since goleak verified all running goroutines, and in parallel tests, the end of a single test may still have many other goroutines running from other tests.

@zchee
Copy link
Contributor Author

zchee commented Jun 5, 2018

@prashantv Thanks for the polite answer. I totally agreed with your thought.
I wait for merge #25 pull request :D

@houqp
Copy link

houqp commented Jun 5, 2018

I think that's reasonable and the behavior should probably be documented :)

In our cases, the tests we run in parallel don't create extra go routines at all, so goleak should still be able to handle it. That said, I think our use-case is probably a pretty restricted edge-case ;)

@bwplotka
Copy link

Is t.Parrallel() something reasonable as a feature request though?

It would be really nice to join forces to finally have solution to this: detection of leak go routines and parallel tests. (:

Related discussion: fortytw2/leaktest#4

@prashantv
Copy link
Collaborator

I don't think there is a good solution for detecting goroutine leaks from a test when using t.Parallel since it's intentionally running with other tests which will definitely have their own running goroutines, and we cannot identify which goroutines are started by which test.

However, there is a workaround: Use the TestMain integration in the package,

func TestMain(m *testing.M) {
	goleak.VerifyTestMain(m)
}

This will verify that there's no leaks after all tests are run.

JacobOaks added a commit to JacobOaks/goleak that referenced this issue Oct 12, 2023
It's a known issue that goleak is incompatible with tests being run in parallel.
* uber-go#16
* uber-go#83

Unfortunately, there is no real solution to this issue.
* uber-go#16 (comment)
* uber-go#83 (comment)
* uber-go#83 (comment)

This PR at least documents the incompatibility
and suggests using `VerifyTestMain` instead.
sywhang pushed a commit that referenced this issue Oct 12, 2023
It's a known issue that goleak is incompatible with tests being run in
parallel.
* #16
* #83

Unfortunately, there is no real solution to this issue.
* #16 (comment)
* #83 (comment)
* #83 (comment)

This PR at least documents the incompatibility
and suggests using `VerifyTestMain` instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants