tests: update + enable check for leaked goroutines #4026
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Works fine locally, let's see what semaphore has to say.. |
|
@jonboulle I think go 14 might still fail? The leaky issue is fixed in go 15 if I remember correctly. I will double check. |
|
Semaphore is fine and I'm using 1.4. What's the leak you have in mind? readloop? |
|
@jonboulle This golang/go#10457 is fixed in 15. |
|
@xiang90 how about this? |
Go 1.4 landed a new testing.M type [1][1] which allows for start-up and shutdown hooks when running tests. The standard library now uses this for checking for leaked goroutines in net/http [2][2]. This patch essentially re-ports the updated code from the net/http test (we were using an older version of it) - in detail: - updates the test to use `TestMain` instead of relying on `TestGoroutinesRunning` to be implicitly run after all other tests - adds a few new goroutines to the list of exceptions (the test itself, as well as the golang/glog package and pkg/log.MergeLogger, both of which spin off goroutines to handle log flushing/merging respectively) - removes a couple of TODOs in the test for extra goroutines that's run after individual tests (one of these re-enables the http package's `.readLoop` and the other was an out-of-date TODO) - re-enables the test [1]: https://golang.org/pkg/testing/#M [2]: https://github.com/golang/go/blob/release-branch.go1.4/src/net/http/main_test.go#L18
| func init() { | ||
| var major, minor int | ||
| var discard string | ||
| i, err := fmt.Sscanf(runtime.Version(), "go%d.%d%s", &major, &minor, &discard) |
There was a problem hiding this comment.
go tip version does not follow this and it is greater than 1.5. but i am fine with not testing go tip.
|
LGTM! Thanks for fixing this! |
jonboulle
added a commit
that referenced
this pull request
Dec 20, 2015
tests: update + enable check for leaked goroutines
jonboulle
added a commit
to jonboulle/rkt
that referenced
this pull request
Dec 22, 2015
Go 1.4 landed a new testing.M type [1][1] which allows for start-up and shutdown hooks when running tests. The standard library now uses this for checking for leaked goroutines in net/http [2][2]. This patch re-ports the updated code from the net/http test. For a previous example, see etcd-io/etcd#4026 [1]: https://golang.org/pkg/testing/#M [2]: https://github.com/golang/go/blob/release-branch.go1.4/src/net/http/main_test.go#L18
jonboulle
added a commit
to jonboulle/rkt
that referenced
this pull request
Dec 23, 2015
Go 1.4 landed a new testing.M type [1][1] which allows for start-up and shutdown hooks when running tests. The standard library now uses this for checking for leaked goroutines in net/http [2][2]. This patch re-ports the updated code from the net/http test. For a previous example, see etcd-io/etcd#4026 [1]: https://golang.org/pkg/testing/#M [2]: https://github.com/golang/go/blob/release-branch.go1.4/src/net/http/main_test.go#L18
jonboulle
added a commit
to jonboulle/rkt
that referenced
this pull request
Jan 19, 2016
Go 1.4 landed a new testing.M type [1][1] which allows for start-up and shutdown hooks when running tests. The standard library now uses this for checking for leaked goroutines in net/http [2][2]. This patch re-ports the updated code from the net/http test. For a previous example, see etcd-io/etcd#4026 [1]: https://golang.org/pkg/testing/#M [2]: https://github.com/golang/go/blob/release-branch.go1.4/src/net/http/main_test.go#L18
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Go 1.4 landed a new testing.M type 1 which allows for start-up and
shutdown hooks when running tests. The standard library now uses this
for checking for leaked goroutines in net/http 2.
This patch essentially re-ports the updated code from the net/http test
(we were using an older version of it) - in detail:
TestMaininstead of relying onTestGoroutinesRunningto be implicitly run after all other testsas well as the golang/glog package and pkg/log.MergeLogger, both of
which spin off goroutines to handle log flushing/merging respectively)
after individual tests (one of these re-enables the http package's
.readLoopand the other was an out-of-date TODO)