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

Jump to bottom

cmd/go: go test -fullpath should include full paths for build errors too #60451

Open
rogpeppe opened this issue May 26, 2023 · 11 comments
Open

cmd/go: go test -fullpath should include full paths for build errors too #60451

rogpeppe opened this issue May 26, 2023 · 11 comments
Labels
GoCommand cmd/go help wanted Proposal Proposal-Accepted
Milestone
Backlog

Comments

@rogpeppe
Copy link
Contributor

What version of Go are you using (go version)?

f90b4cd6554f4f20280aa5229cf42650ed47221d

Does this issue reproduce with the latest release?

N/A

What did you do?

I ran this testscript:

! exec go test -C ./x -fullpath .
! stderr '\./x.go'

-- x/x.go --
package foo

var x = mistake

-- go.mod --
module test

What did you expect to see?

A passing test.

What did you see instead?

> ! exec go test -C ./x -fullpath .
[stdout]
FAIL
[stderr]
# test/x
./x.go:3:9: undefined: mistake
[exit status 1]
> ! stderr '\./x.go'
FAIL: /tmp/testscript142857279/x.txtar/script.txtar:2: unexpected match for `\./x.go` found in stderr: ./x.go

It seems to me that the intent of -fullpath is to cause all filenames to print in a way that's absolute, even when those errors come from the compiler rather than the test binary. I'd expect the compiler error to print an absolute path there too.
@ianlancetaylor
Copy link
Contributor

This means moving -fullpath from being a test option to being a build option, and then using it to pass something to cmd/compile. We would presumably still have the -test.fullpath option, which would be different, but -fullpath would imply -test.fullpath.

CC @bcmills @matloob

@ianlancetaylor ianlancetaylor added the GoCommand cmd/go label May 26, 2023
@rogpeppe
Copy link
Contributor Author

Yup, that sounds about right. I think that would be worth doing.

@bcmills
Copy link
Contributor

bcmills commented May 26, 2023

Sounds reasonable. This is essentially a revision on proposal #37708, so it should probably be a proposal itself.

@bcmills bcmills added this to the Proposal milestone May 26, 2023
@rsc
Copy link
Contributor

rsc commented Jun 14, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Jun 21, 2023

I think this would mean making -fullpath a build flag (as opposed to a test flag) and then have base.ShortPath not shorten when the flag is set. Does anyone object to this?

@rsc
Copy link
Contributor

rsc commented Jun 28, 2023

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Jul 5, 2023

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc modified the milestones: Proposal, Backlog Jul 5, 2023
@quantonganh
Copy link
Contributor

If we add -fullpath as build option, it may lead to confusion with the -trimpath flag:

        -trimpath
                remove all file system paths from the resulting executable.
                Instead of absolute file system paths, the recorded file names
                will begin either a module path@version (when using modules),
                or a plain import path (when using the standard library, or GOPATH).

What do you think if we just pass -test.fullpath to cmd/compile?

@gopherbot
Copy link

Change https://go.dev/cl/535075 mentions this issue: cmd/go/internal/work: go test -fullpath should include full paths for build errors

@bcmills
Copy link
Contributor

bcmills commented Oct 26, 2023

@quantonganh, if -fullpath and -trimpath are both specified, the “full path” logged by the test should correspond to the short filename joined with the package import path.

It isn't clear to me what -fullpath as a build option should do in that case, though. 🤔

@bcmills
Copy link
Contributor

bcmills commented Oct 26, 2023

Also: if -fullpath is (also) a build flag, should it apply uniformly to all build commands? It seems inconsistent to apply it to only the build portion of go test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GoCommand cmd/go help wanted Proposal Proposal-Accepted
Projects
Proposals
Status: Accepted
Milestone
Backlog
Development

No branches or pull requests
6 participants
@rogpeppe @rsc @quantonganh @ianlancetaylor @bcmills @gopherbot