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

Module go download logs appearing as Errors (go v1.11) #27

Closed
jonasdebeukelaer opened this issue Nov 2, 2018 · 8 comments
Closed

Module go download logs appearing as Errors (go v1.11) #27

jonasdebeukelaer opened this issue Nov 2, 2018 · 8 comments
Labels
bug Something isn't working

Comments

@jonasdebeukelaer
Copy link
Contributor

jonasdebeukelaer commented Nov 2, 2018

Hey, I'm using the following command to run tests:

gotestsum -- ./api/...

inside a golang:1.11.1-stretch docker container, and also locally (mac), and getting the following output

∅ bitbucket.com/satalia/engine/api/healthsvc
∅ bitbucket.com/satalia/engine/api/healthsvc/endpoints
✓ bitbucket.com/satalia/engine/api/healthsvc/service (5ms)
∅ bitbucket.com/satalia/engine/api/healthsvc/transport

=== Errors
go: downloading github.com/go-kit/kit v0.7.0
go: downloading github.com/golang/protobuf v1.2.0
go: downloading google.golang.org/grpc v1.15.0
go: downloading golang.org/x/net v0.0.0-20181011144130-49bb7cea24b1
go: downloading google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8
go: downloading golang.org/x/text v0.3.0
go: downloading github.com/go-logfmt/logfmt v0.3.0
go: downloading github.com/go-stack/stack v1.8.0

DONE 1 tests, 8 errors in 29.465s

whereas if I run using go test ./api/... I get none of the go downloading errors.

We are using go modules to manage dependencies (a file called go.mod) contains all the required packages, could this be an issue?

@jonasdebeukelaer jonasdebeukelaer changed the title Errors on go dowloads Errors on go downloads Nov 2, 2018
@dnephin
Copy link
Member

dnephin commented Nov 2, 2018

What happens if you run go test -json ./api/... > /dev/null do you see those lines on stderr? Another thing to check would be running go test from a fresh checkout without any dependencies downloaded.

I think what's happening is that in go 1.10 any stderr indicated an error. Now with go 1.11 there are go module downloads happening, and those lines are printed to stderr so the previous rule of "all stderr is an error" no longer works. This is unfortunate and is probably worth reporting to the go team while go modules are still in beta.

As a workaround I think the stderr handler can check for a prefix match of go: downloading and ignore those lines. Possibly only when they appear as the first lines of stderr.

That patch would go somewhere around here: https://github.com/gotestyourself/gotestsum/blob/master/testjson/execution.go#L320. If you are interested in submitting a PR to fix that would be awesome!

@dnephin dnephin added the bug Something isn't working label Nov 2, 2018
@jonasdebeukelaer
Copy link
Contributor Author

Hey dnephin thanks for the quick response. I realised I understood what was going on completely wrong (I thought there were actual errors around downloading, but these were somehow not failing the tests...) anyway, that makes sense then.

And yes when piping the json out you still get go downloading or go extracting lines appearing in the terminal output (I'm assuming that means they are understood as stderr as you mention?). Does seem like that shouldn't be stderr...

Yeah might have a quick stab at patching that then, cheers

@jonasdebeukelaer jonasdebeukelaer changed the title Errors on go downloads Module go download logs appearing as Errors (go v1.11) Nov 2, 2018
@jonasdebeukelaer
Copy link
Contributor Author

hey I've created the fix, do you need to give me access? cheers

@dnephin
Copy link
Member

dnephin commented Nov 2, 2018

Thanks for working on a fix!

To submit a Pull Request, first fork the repo, and push the change to your fork. When you do that you should be presented a link to create a PR in the output of git push. You'll also see a banner on github when you visit https://github.com/gotestyourself/gotestsum to create the PR. https://help.github.com/articles/creating-a-pull-request-from-a-fork/ has some more details.

@jonasdebeukelaer
Copy link
Contributor Author

Cheers for the info, and sorry for being a total noob. Shamefully the first time contributing to an open source project 🤦‍♂️

@dnephin
Copy link
Member

dnephin commented Nov 3, 2018

No problem! I hope that your first experience contributing to open source was a positive one!

@jonasdebeukelaer
Copy link
Contributor Author

Yeah all good thanks :)
Also, when do you think you will release these changes? Cheers

@jonasdebeukelaer
Copy link
Contributor Author

I've just seen you did release, just Google Cloud Build wasn't picking that up for some reason. Ignore me 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants