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: testing: test-scoped context #58211

Closed
AdamNorberg opened this issue Feb 1, 2023 · 2 comments
Closed

Proposal: testing: test-scoped context #58211

AdamNorberg opened this issue Feb 1, 2023 · 2 comments

Comments

@AdamNorberg
Copy link

In the documentation for the context package, "tests" are listed as one of the common uses of context.Background(). Because tests exist for a very specific amount of time and, typically, we would like test resources to be cleaned up at the end of their tests, it seems to me that tests should have a context.Context available that becomes canceled when the test completes (for any reason).

I propose that testing.TB adds a new method, Ctx() context.Context, which returns a Context with the same scope as the current test, subtest, or parallel test; it will cancel immediately after its batch of Cleanup operations finishes, as though it was on the Cleanup stack before control passed to the test.

This assists test cleanup by giving tests that spin off goroutines performing cancellation-aware operations a better chance of cleaning themselves up if a test ends abruptly (t.FailNow and associated, panic, etc.) or contains bugs in its cleanup logic. It also removes a case where developers currently use context.Background(), reinforcing habits of avoiding it everywhere outside main. It is conceptually coherent with the idea of the testing and context packages, and not having a test-associated context feels a bit weird to me now that I am very used to context plumbing just about everywhere.

This feature is convenient but not necessary. It is functionally identical to starting each test or subtest with

ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)

and using ctx as the "root context" for the test thereafter. It occurred to me that if I find myself doing this in many different tests to assist scoping and cleanup, maybe testing.T (or B, or F) should offer it as a feature.

This introduces compatibility issues with any code that refers to testing.TB and implements its own implementation, which would not have the Ctx() method. This could be avoided by omitting Ctx from testing.TB, but implementing it on the published types anyway.

Another alternative, which also avoids backwards-compatibility issues, is to recognize function signatures like TestXxx(context.Context, *testing.T) as tests (with equivalents for benchmarks and fuzzing). (*T).Run and related methods would need RunWithCtx equivalents expecting subtests with this signature. This approach matches the standard pattern for context plumbing in Go, at the expense of the modestly clumsy RunWithCtx and added complexity in describing just what a test function is.

@gopherbot gopherbot added this to the Proposal milestone Feb 1, 2023
@rittneje
Copy link

rittneje commented Feb 1, 2023

This introduces compatibility issues with any code that refers to testing.TB and implements its own implementation, which would not have the Ctx() method.

Note that testing.TB includes a private method, meaning that it is not possible for anything outside the testing package to implement it (other than via anonymous embedding tricks). That is why it was safe to add things like Helper() and TempDir() in the past.

@ianlancetaylor
Copy link
Contributor

Thanks, but closing as dup of #36532.

@ianlancetaylor ianlancetaylor closed this as not planned Won't fix, can't repro, duplicate, stale Feb 1, 2023
@golang golang locked and limited conversation to collaborators Feb 1, 2024
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