Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

Allow version-tagged filepaths in tests #167

Open
thsnr opened this issue Aug 18, 2018 · 5 comments
Open

Allow version-tagged filepaths in tests #167

thsnr opened this issue Aug 18, 2018 · 5 comments
Milestone

Comments

@thsnr
Copy link

thsnr commented Aug 18, 2018

(Continued from PR #165).

Description

Go 1.11 will introduce support for versioned modules. When running in module-aware mode, if your package depends on github.com/pkg/errors then Go will download it into a .../github.com/pkg/errors@v0.8.0/ path (or whatever the requested version). This makes format_test.go and stack_test.go fail during go test all because they expect the filepaths to contain /github.com/pkg/errors/.

Steps to reproduce

  1. Go into an empty directory NOT on GOPATH.
  2. go1.11rc1 mod init github.com/thsnr/gomod.
  3. go1.11rc1 get github.com/pkg/errors@v0.0.0-00000000000000-816c9085562cd7ee03e7f8188a1cfd942858cded (must use pinned commit because 0.8.0 fails with another error fixed in master).
  4. go1.11rc1 test github.com/pkg/errors (usually run during go test all).

Expected results

All tests pass.

Actual results

Some tests fail because they are on a github.com/pkg/errors@v<version> path:

format_test.go:379: test 2: line 2: fmt.Sprintf("%+s", err):
     got: "github.com/pkg/errors.init\n\t/home/thsnr/go/pkg/mod/github.com/pkg/errors@v0.0.0-00000000000000-816c9085562cd7ee03e7f8188a1cfd942858cded/stack_test.go"
    want: "github.com/pkg/errors.init\n\t.+/github.com/pkg/errors/stack_test.go"

The PR simply proposed allowing version tags in the filepaths in those tests. It was closed because of "a broader problem with assuming the prefix is stable" and "the logic to trim the compile time GOPATH from source file paths" no longer working.

However I am unsure how these issues are relevant as the github.com/pkg/errors package does not assume a stable prefix outside of these tests and no GOPATH trimming is performed: it simply prints the filename reported by Func.FileLine (although see #166).

Opening this issue to continue the discussion from #165.

@davecheney
Copy link
Member

I don't think it's going to be possible to support the go test all use case and I'm not sure that the notion of testing all the packages makes sense in a post GOPATH world.

I know there is a bug related to tidying up the prefix reported by Func.FileLine, but I don't think solving the go test all use case is the answer.

@davecheney davecheney added this to the 1.0.0 milestone Jan 6, 2019
@simonklb
Copy link

simonklb commented Feb 4, 2019

What is the argument against running go test all in a post GOPATH world? To me it makes a lot of sense to test all of your dependencies, especially before a release. Also see: golang/go#26317 (comment)

@davecheney
Copy link
Member

Thank you for highlighting golang/go#26317 , I'll have to change the tests to work in this manner. I don't have an ETA for that, patches welcome.

@anacrolix
Copy link

I had a go at this; I didn't find a better way to determine the correct source paths in the stack traces than the one given in #165. Without a better way, I think #165 is right on the money. vgo test all is also encouraged. I was able to add a test specifically for vgo test all, but it required embedding a "test" module inside this repository, and would have no value unless running vgo test all against that module was added to the CI to prevent breakages.

@mbyio
Copy link

mbyio commented Jul 16, 2019

Hey, I just want to add an explanation for why I think supporting go test all is important: go test all allows us to verify that dependencies, especially child dependencies, work together as expected. Whenever a new dependency is added, we can run go test all to quickly see that things are generally "ok". Currently, this is the only library I use that has consistent test failures when run with go test all.

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

No branches or pull requests

5 participants