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: set an environment variable to indicate we're in a test #66122

Closed
matloob opened this issue Mar 5, 2024 · 6 comments
Closed
Labels
Milestone

Comments

@matloob
Copy link
Contributor

matloob commented Mar 5, 2024

Proposal Details

This proposal is to have the testing package set a new GO_TESTING environment variable, perhaps for the duration of testing.(*M).Run, to indicate that it's running a test. This would allow subprocesses started by the test to know that they're running as part of a test.

This is useful so that the Go toolchain could suppress telemetry when it's invoked by a test to avoid contaminating users' telemetry data.

@bcmills @adonovan @findleyr

@gopherbot gopherbot added this to the Proposal milestone Mar 5, 2024
@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Mar 5, 2024
@bcmills
Copy link
Contributor

bcmills commented Mar 5, 2024

I wonder if the variable should contain some more metadata — perhaps the package path under test? (The test can theoretically infer that itself from its working directory, but if the test starts a go subprocess in, say, a temp directory, that information may be lost.)

Separately, I have been wondering whether go test should also set a GOMOD and/or GOWORK environment variable to allow cmd/go to reliably find the module root from which the test's dependencies were computed, so that it doesn't end up running in a completely different configuration if invoked as a subprocess of a test of a dependency.

@cespare
Copy link
Contributor

cespare commented Mar 5, 2024

This partially duplicates the testing.Testing function. I understand that you want something that a subprocess could use so it's not useful here, but my point is that if GO_TESTING had existed we probably wouldn't have added testing.Testing, right?

Is this something that would be broadly useful, given testing.Testing? ISTM that needing to know whether we're running under a test in a subprocess isn't a common thing to need, and if people need it for specific tests they can already work around it today pretty easily by using testing.Testing and then passing that information to subprocesses using whatever env variable, flag, etc. that they want.

If this is mostly just needed for the go command itself, is there a more narrowly tailored way for it to get that information?

This would allow subprocesses started by the test to know that they're running as part of a test.

(Only subprocesses that are created with an inherited environment.)

@findleyr
Copy link
Member

findleyr commented Mar 5, 2024

Thanks for filing this issue. For a motivating example: gopls invokes the Go command thousands of times during its test suite, on tiny workspaces, with various non-standard permutations of settings. We don't want these invocations to drown out or pollute data collected from "real" uses of the Go command, either directly from the command line or during a real gopls session. And while there may be few developers working on gopls, there are many other tools that integrate with the Go command in tests. I think we need some sort of across-the-board solution like this, though I don't feel strongly about the details of the solution.

@bcmills
Copy link
Contributor

bcmills commented Mar 5, 2024

The flip-side of that is: if there are features of the Go toolchain that are primarily useful in tests, and users are actually relying on them there, then it would be nice to have telemetry for that usage so that we don't incidentally break those tests.

@matloob
Copy link
Contributor Author

matloob commented Mar 6, 2024

I agree with @findleyr that we need some sort of across-the-board solution and I also don't feel strongly about the details of the solution.

The reason why testing.Testing doesn't solve our particular problem is that every test that ends up invoking the go command has to be aware of the problem and do work to communicate to the go command that it's being invoked from a test context.

@matloob
Copy link
Contributor Author

matloob commented Mar 6, 2024

I'll retract this proposal. We've decided it's okay to capture data from testing.

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

No branches or pull requests

5 participants