Skip to content

x/telemetry/godev/cmd/telemetrygodev: TestConfig fails when the ENV environment variable is set #62138

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

Closed
bcmills opened this issue Aug 18, 2023 · 1 comment
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. telemetry x/telemetry issues
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Aug 18, 2023

https://build.golang.org/log/01632ff0d1e2a2a8aeeca54449561a80220e9296:

--- FAIL: TestConfig (0.00s)
    config_test.go:25: Config() = &{8080 go-telemetry localhost:8081 .localstorage /root/.shrc-telemetry-uploaded ../config/config.json 102400 10m0s false false}, want &{8080 go-telemetry localhost:8081 .localstorage local-telemetry-uploaded ../config/config.json 102400 10m0s false false}
FAIL
FAIL	golang.org/x/telemetry/godev/cmd/telemetrygodev	0.124s

This appears to be due to the use of the ENV environment variable for configuration:
https://go.googlesource.com/telemetry/+/b0bf2e6e269ce288c82914e7d126bfe345aa84e9/godev/cmd/telemetrygodev/config.go#75

Per https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sh.html#tag_20_117_08, the standard meaning of the ENV variable is:

ENV
This variable, when and only when an interactive shell is invoked, shall be subjected to parameter expansion (see Parameter Expansion) by the shell, and the resulting value shall be used as a pathname of a file containing shell commands to execute in the current environment. The file need not be executable. If the expanded value of ENV is not an absolute pathname, the results are unspecified. ENV shall be ignored if the real and effective user IDs or real and effective group IDs of the process are different.

If godev/cmd/telemetrygodev needs a configuration variable to use for a different, Go-specific purpose, it should use a Go-specific variable name. (And probably the same is true for the PORT variable. 😅)

(attn @jamalc @hyangah @pjweinb)

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 18, 2023
@gopherbot gopherbot added the telemetry x/telemetry issues label Aug 18, 2023
@gopherbot gopherbot added this to the Unreleased milestone Aug 18, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/521595 mentions this issue: godev/cmd: use GO_TELEMETRY_ENV instead of ENV

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. telemetry x/telemetry issues
Projects
None yet
Development

No branches or pull requests

2 participants