x/tools/internal/testenv: stop parsing CombinedOutput #65729
Assignees
Labels
Milestone
Comments
gopherbot
added
the
Tools
|
Change https://go.dev/cl/564515 mentions this issue: |
|
Change https://go.dev/cl/564516 mentions this issue: |
|
Change https://go.dev/cl/564339 mentions this issue: |
gopherbot
pushed a commit
to golang/tools
that referenced
this issue
Feb 16, 2024
I fumbled with git and forked the CL, stranding some review comments from bcmills on the ill-fated fork. Updates golang/go#65729 Change-Id: I6d0bf431f841dacb94e9e13a90bf39f8e2ed2fbf Reviewed-on: https://go-review.googlesource.com/c/tools/+/564339 Reviewed-by: Bryan Mills <bcmills@google.com> Auto-Submit: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
apstndb
pushed a commit
to apstndb/gotoolsdiff
that referenced
this issue
Jan 11, 2025
This change fixes a bug in internal/testenv in which it would gather the combined output (2>&1) of the "go env" command and parse it as an environment variable. However, certain environment variables (e.g. GODEBUG) cause "go env" to log to stderr, so that the parser reads garbage. Use Output instead. Also, preemptively fix a number of similar occurrences in x/tools. CombinedOutput should be used only when the whole output is ultimately sent to stderr or a log for human eyes, or for tests that look for specific error messages in the unstructured combined log. In those cases, the scope of the 'out' variable can be reduced to avoid temptation. Fixes golang/go#65729 Change-Id: Ifc0fd494fcde0e339bb5283e39c7696a34f5a699 . Change-Id: I6eadd0e76498dc5f4d91e0904af2d52e610df683 Reviewed-on: https://go-review.googlesource.com/c/tools/+/564516 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Findley <rfindley@google.com>
apstndb
pushed a commit
to apstndb/gotoolsdiff
that referenced
this issue
Jan 11, 2025
I fumbled with git and forked the CL, stranding some review comments from bcmills on the ill-fated fork. Updates golang/go#65729 Change-Id: I6d0bf431f841dacb94e9e13a90bf39f8e2ed2fbf Reviewed-on: https://go-review.googlesource.com/c/tools/+/564339 Reviewed-by: Bryan Mills <bcmills@google.com> Auto-Submit: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The testenv package defines a number of helpers for skipping tests. One of them checks that
go env GOROOTandruntime.GOROOTagree; if not, it skips the test. (Perhaps it should fatal? But I digress.)When running with GODEBUG=countertrace, the go env command prints a bunch of stuff to stderr, as it should. However, testenv runs the command and then gathers its CombinedOutput (
2>&1), which means it is no longer a well-formed go env result. Mayhem ensues.No Go program should attempt to parse CombinedOutput. It is for human eyes only (or tests that look for fragments of error messages among the wreckage).
The text was updated successfully, but these errors were encountered: