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

test: avoid leaking goroutines #124

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Conversation

trym-b
Copy link
Contributor

@trym-b trym-b commented Apr 2, 2024

This commit adds a test that checks for goroutine
leaks in the workerpool package, and also fixes
two goroutine leaks.

There is unfortunately no easy way of running
goleak before every test without either a lot of copy-paste, or additional code to ensure every
test file has a TestMain function.

A leaked goroutine will produce the following
error with goleak:

goleak: Errors on successful test run: found
unexpected goroutines: [Goroutine 7 in state chan
receive, with
github.com/nlnwa/warchaeology/internal/workerpool
.New.func1 on top of the stack: goroutine 7 [chan
receive]:
github.com/nlnwa/warchaeology/internal/workerpool.
New.func1(0xc00008a1e0)
./warchaeology/internal/workerpool/workerpool.go:
23
+0x66 created by
github.com/nlnwa/warchaeology/internal/workerpool.New
in goroutine 6
./warchaeology/internal/workerpool/workerpool.go:
22
+0x85 ] FAIL
github.com/nlnwa/warchaeology/internal/workerpool
0.450s FAIL

This commit adds a test that checks for goroutine
leaks in the `workerpool` package, and also fixes
two goroutine leaks.

There is unfortunately no easy way of running
`goleak` before every test without either a lot of
copy-paste, or additional code to ensure every
test file has a `TestMain` function.

A leaked goroutine will produce the following
error with `goleak`:

```
goleak: Errors on successful test run: found
unexpected goroutines: [Goroutine 7 in state chan
receive, with
github.com/nlnwa/warchaeology/internal/workerpool
.New.func1 on top of the stack: goroutine 7 [chan
receive]:
github.com/nlnwa/warchaeology/internal/workerpool.
New.func1(0xc00008a1e0)
./warchaeology/internal/workerpool/workerpool.go:
23
+0x66 created by
github.com/nlnwa/warchaeology/internal/workerpool.New
in goroutine 6
./warchaeology/internal/workerpool/workerpool.go:
22
+0x85 ] FAIL
github.com/nlnwa/warchaeology/internal/workerpool
0.450s FAIL
```
@trym-b trym-b force-pushed the test/add-goleak-to-relevant-tests branch from 4063436 to 67ab417 Compare April 2, 2024 16:27
@trym-b trym-b merged commit 30f82bc into main Apr 2, 2024
8 checks passed
@trym-b trym-b deleted the test/add-goleak-to-relevant-tests branch April 2, 2024 18:08
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

Successfully merging this pull request may close these issues.

2 participants