Skip to content

testing: unify context usage in tests #14220

@nvb

Description

@nvb

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.

cc. @andreimatei @RaduBerinde @tamird

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-cleanupTech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions