-
Notifications
You must be signed in to change notification settings - Fork 4.1k
testing: unify context usage in tests #14220
Description
See discussion in #14128.
Currently, we have a mixture of context.Background and context.TODO calls scattered throughout our tests. It's unclear which one to use, and no pattern has formed. There are good arguments for and against using both. context.Background is the standard recommended usage for tests and is the convention adopted by most Go project, but implies that the context usage is a resolved question. Meanwhile, context.TODO implies that future work is desired and helps keep unresolved context decisions separate from fully resolved ones, but is primarily meant for locations which we plan on fixing later by plumbing in a real context.
@RaduBerinde further elaborated on this:
I used TODO in tests because I thought there might be a good reason to some day have a
per-test context (like the proposed testutils.Context(t)) for capturing traces. The appdash
work mentioned is one potential reason for this. We may also want to run various tests
with Lighstep if it helps debugging. I think being able to look at a lightstep trace when
starting to debug a test failure could save a lot of time.
We should either fully adopt context.Background in tests as recommended by the Go documentation or replace all contexts in tests with some per-test context. Either way, context.TODO should not sit in our tests forever.