Skip to content

proposal: testing: add additional data to testing.T.Context #70480

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

Open
adam-azarchs opened this issue Nov 21, 2024 · 3 comments
Open

proposal: testing: add additional data to testing.T.Context #70480

adam-azarchs opened this issue Nov 21, 2024 · 3 comments
Labels
Milestone

Comments

@adam-azarchs
Copy link
Contributor

Proposal Details

The accepted proposal #36532 added a Context method to testing.T, which is great, but I can think of two straightforward ways to improve its usefulness further:

  1. Expose T.Deadline() as the deadline for the context. There are (hopefully rare) cases where something needs to propagate deadline information to something that isn't go, e.g. a subprocess, where simple cancelation may not be sufficient. Even in cases where the final consumer of the context is go, it's helpful for e.g. logging to be able to distinguish between a context cancelled due to a timeout vs. cancelled for some other reason. This should be as simple as swapping out context.WithCancel for context.WithDeadline where appropriate.
  2. Create a trace.Task named for the test. When running a trace or benchmark with -trace, it can be handy to be able to track back regions to specific tests. This, too, is fairly low-cost given that the testing package already depends on runtime/trace.
@gopherbot gopherbot added this to the Proposal milestone Nov 21, 2024
@smithamax
Copy link

Expose T.Deadline() as the deadline for the context

Now that T.Context() is out, wouldn't this potentially break existing tests that check for the existence of a deadline. Would have been nice if it was in there from the start, but I don't think it can be added now

@adam-azarchs
Copy link
Contributor Author

The presence or absence of a deadline isn't specified in the T.Context() API; undefined behavior is fair game for changes. Alternatively, if this is deemed a serious issue, then we do now have the ability to conditionalize the behavior based on the go version declared in the go.mod. It seems very unlikely that setting the deadline would in itself change the behavior of called code.

The main use cases I'd see for setting the deadline:

  1. Changes (improves) some error messages in a few cases. "Context canceled" is less clear what happened than "timed out".
  2. There are some standard library APIs which don't take a context.Context for cancelation where you'd really like to have stuff abort if things time out. For example, maybe those on http.ResponseController.
  3. CGO calls to functions which may accept a deadline but can't use context.Context because they're not go.

I suppose it's true that one could always just put in

ctx = t.Context()
if d, ok := t.Deadline(); ok {
    var cancel context.CancelFunc
    ctx, cancel := context.WithDeadline(ctx, d)
    defer cancel()
}

but aside from being simply awkward, it's also not quite the same behavior because of the need to deal with the CancelFunc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

4 participants