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

Proposal: add context.Context objects to testing.T EDIT: and testing.M #18182

Closed
Lucretiel opened this issue Dec 3, 2016 · 6 comments
Closed

Comments

@Lucretiel
Copy link

Proposal: add a test-local Context object to testing.T objects. The object would be accessible like this:

func TestSomething(t *testing.T) {
    ctx = t.Context()
    ...
}

It would be cancelled at the end of the test. This would allow tests involving network calls, parallel goroutines, or other blocking code to use the context to cancel all the background stuff in the event the test fails an assertion early with FailNow or something similar.

It should be a fairly straightforward feature, so if there's interest I can create a Pull Request when I have time.

@minux
Copy link
Member

minux commented Dec 3, 2016 via email

@Lucretiel
Copy link
Author

Right you are! I wasn't looking at tip.. Ty!

@bradfitz
Copy link
Contributor

bradfitz commented Dec 5, 2016

@Lucretiel, what was your motivation for wanting this, out of curiosity?

@Lucretiel
Copy link
Author

Lucretiel commented Dec 5, 2016

I have an internal RpcClient struct that uses a background goroutine to keep the connection pool up to date– closing idle connections, performing new host discovery, and so on. We use a Context to tell this object when to close that goroutine. It's fairly straightforward to do WithCancel and defer cancel(), but it seems like Context is becoming the de-facto mechanism to "signal a collection of goroutines to stop working and shut down," so it'd be nice to have it provided by test to prevent duplicating those lines.

Originally, my request was to add the feature to gocheck (go-check/check#88). In particular, I thought it'd be nice to have a single, suite-level Context, as well as individual test-level Contexts. Since gocheck is based on testing, I figured I'd request it here, too.

Actually, since I just discovered that testing also supports suite-level fixtures through TestMain, I wonder if there would be any interest in adding a Context to testing.M, and having the individual testing contexts be its children?

@Lucretiel Lucretiel reopened this Dec 5, 2016
@Lucretiel Lucretiel changed the title Proposal: add context.Context objects to testing.T Proposal: add context.Context objects to testing.T EDIT: and testing.M Dec 5, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Dec 5, 2016

This was already added to Go master (to-be Go 1.8). There are concerns about it, though. Whether to keep it is being tracked in #18199.

I'm going to close this bug again as it's a dup of #16221 and #18199

@bradfitz bradfitz closed this as completed Dec 5, 2016
@Lucretiel
Copy link
Author

Sounds good. Ty for the update.

@golang golang locked and limited conversation to collaborators Dec 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants