go icon indicating copy to clipboard operation
go copied to clipboard

proposal: testing: optionally include full (or relative) path name

Open stapelberg opened this issue 6 years ago • 6 comments

Previous discussion happened in issue #21119.

Currently, t.Error calls print only the base name of the source file.

This is fine when running a single test, but in larger projects, it is a significant productivity boost to have relative (or full) path names, which make the “jump to next compilation error” feature in popular editors work.

I understand the outcome of #21119 (can’t change the default), but I suggest adding a knob to opt into printing relative or full paths.

Perhaps this could be similar to log.SetFlags, where you can specify Llongfile or Lshortfile?

Let me know if this would be okay, and I’d be happy to send a CL.

stapelberg avatar Mar 06 '20 07:03 stapelberg

@rogpeppe, @myitcv and myself discussed a very similar idea not too long ago.

It's unfortunate that we can't change the default. However, there is some precedent for such an opt-in flag:

$ go tool compile -h 2>&1 | grep -- -L
  -L	show full file names in error messages

I also wonder if go test could transform filename paths transparently, when it buffers output from test binaries. The only case where the test binary output goes straight to the terminal is when go test is run without arguments - and, in that case, the paths are already relative to the current directory, as we're testing the package in the current directory too. Transforming paths should be possible to do, given that test2json does very similar parsing of plaintext test output.

mvdan avatar Mar 06 '20 13:03 mvdan

also ccing @mpvl

toothrot avatar Mar 09 '20 17:03 toothrot

Is there anyway we can do this similar to how the log package sets flags for log outputs?

// setting longfile in package log.
log.SetFlags(log.Lmicroseconds | log.Llongfile)

Proposed:

testing.SetFlags(testing.Llongfile)

aryman-glympse avatar Jun 11 '21 20:06 aryman-glympse

#55976 was closed as a duplicate of this issue, but this one seems to be stuck in the backlog. Can we turn this into a proposal?

Moving my comment here for consideration:

I've noticed occasionally GoLand links to the wrong file when tests are run from the terminal window inside GoLand. I suspect it's for exactly the reason described above. The filename is ambiguous. There can be multiple files with the same name in a project, and when tests are run from the terminal window, it doesn't know which of those files to link to. An absolute path would allow this feature to work reliably.

This problem exists even when t.Helper is used correctly. The tooling that is reading the output doesn't necessarily know which directory is the current working directory because tests can be run from any directory.

I suspect the absolute path is really only useful when the output is being read by another application, and the filename is better when the output is being read by a human. Instead of a new flag to enable this behaviour, the existing -json flag on go test could be used to enable output of absolute paths instead of filenames.

https://github.com/golang/go/commit/1c72ee7f13831b215b8744f6b35bc4fd53aba5e2 recently added -test.v=json, which I think allows testing to detect if -json is set or not.

dnephin avatar Dec 17 '22 19:12 dnephin

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 avatar Dec 21 '22 19:12 rsc

Excited to see that the proposal will be reviewed at the weekly proposal review meeting!

It would be great if there was an option to print the full/relative path of the source file even when the test succeeds.

For my company's repos, I'm working on associating tests with owners (using a CODEOWNERS file within the repo). We have large packages that multiple teams contribute too, so it's impossible to assign ownership based on package unless we take on a large amount of refactoring work. It's fairly easy however to assign ownership by files.

I'm able to assign ownership to a testcase when it fails based on the filename printed, however, I haven't been able to assign ownership for tests that pass. Printing the full/relative path of the source file for every test run would enable this.

jmhwang7 avatar Dec 22 '22 15:12 jmhwang7

I think the most reasonable way to do this would be to add an option to go test. That would then permit cases like go test ... to report full path names for testing log messages, which is convenient if there are a lot of packages with similar file names.

So what should the option be? I suggest -test.fullpath, and perhaps go test should recognize -fullpath and convert it as it does with several other options.

ianlancetaylor avatar Jan 04 '23 20:01 ianlancetaylor

Does anyone object to adding -fullpath as a test option for this?

rsc avatar Jan 11 '23 18:01 rsc

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

rsc avatar Jan 18 '23 19:01 rsc

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

rsc avatar Jan 25 '23 19:01 rsc

Great! Does anyone want to contribute a CL for this? Otherwise I can have a look, but it might be a few weeks until I find the time.

stapelberg avatar Jan 25 '23 19:01 stapelberg

@stapelberg, I've prepared a CL for this, and I'll send it in the next few days.

empire avatar Jan 26 '23 20:01 empire

This will help vscode go extension's job much easier. (vscode runs go test or dlv test from the open file's directory). Can we extend this flag apply to produce absolute path names for build errors? (-gcflags=all=-L should do it in theory, but it doesn't do it now) That way, when a test fails due to build error, the full path will be shown with the build errors.

hyangah avatar Jan 26 '23 20:01 hyangah

Change https://go.dev/cl/463837 mentions this issue: testing: add -fullpath to go test

gopherbot avatar Jan 27 '23 15:01 gopherbot

I can confirm that setting -fullpath works as expected in Emacs’s compilation-mode :)

% ~/upstream-go/bin/go test -fullpath -count=1 -run=/sqlite -v ./...
[…]
=== RUN   TestUpdate/sqlite
2023/02/23 19:27:28 using database: txdb/sqlite
2023/02/23 19:27:28 Ingested image "abcdefg" (matching "scan2drive")
2023/02/23 19:27:28 Setting desired image for machine "scan2drive" to "abcdefg"
    /home/michael/go/src/github.com/gokrazy/gus/internal/gusserver/update_test.go:61: update: diff (-want +got):
          gusapi.UpdateResponse{
          	SbomHash:     "abcdefg",
        - 	RegistryType: "localdiska",
        + 	RegistryType: "localdisk",
          	DownloadLink: "/doesnotexist/disk.gaf",
          }

Pressing M-g n brings me into the correct file and line 🎉

Thanks for getting this implemented :)

stapelberg avatar Feb 23 '23 18:02 stapelberg

Change https://go.dev/cl/498396 mentions this issue: doc/go1.21: mention new go test -fullpath option

gopherbot avatar May 25 '23 21:05 gopherbot