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

x/build: make misc-compile-all trybot type check tests too #20119

Closed
bradfitz opened this issue Apr 25, 2017 · 9 comments
Closed

x/build: make misc-compile-all trybot type check tests too #20119

bradfitz opened this issue Apr 25, 2017 · 9 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge
Milestone

Comments

@bradfitz
Copy link
Contributor

The "misc-compile-*" Trybot builders try to at least compile & type check every GOOS/GOARCH pair, even when we don't have sufficient hardware to run the tests.

But they apparently don't compile or type check tests.

https://go-review.googlesource.com/c/41691/ broke Plan 9 without any warnings.

Fix that.

/cc @mvdan @kevinburke @adams-sarah @cybrcodr

@gopherbot gopherbot added this to the Unreleased milestone Apr 25, 2017
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Apr 25, 2017
@gopherbot
Copy link

CL https://golang.org/cl/41752 mentions this issue.

gopherbot pushed a commit that referenced this issue Apr 25, 2017
Cleanup CL https://golang.org/cl/41691 broke the plan9 build by removing
a use of a package but not removing the package import.

Trybots don't check that. I filed #20119 for that.

Change-Id: Ia030e6924665dfb871ca964455b899d51b0200c2
Reviewed-on: https://go-review.googlesource.com/41752
Reviewed-by: David du Colombier <0intro@gmail.com>
@bradfitz
Copy link
Contributor Author

bradfitz commented May 2, 2017

Notes:

The misc builders are defined here:
https://github.com/golang/build/blob/60446e188e72edd4658dc6d9f6e3c1b9f5227014/dashboard/builders.go#L809

They use buildall.bash to run: (ignore the typo in docs that says buildall.sh)
https://github.com/golang/build/blob/60446e188e72edd4658dc6d9f6e3c1b9f5227014/dashboard/builders.go#L578
https://github.com/golang/go/blob/master/src/buildall.bash

All that basically does (from a fast linux-amd64 kubernetes container) is:

$ GOOS=something GOARCH=something build -a std cmd

We'd also need a way to also make buildall.bash run "go test -c $PKG" for each $PKG. That is #15513.

@paranoiacblack
Copy link
Contributor

Yeah, I tried just modifying buildall.bash to run $GOPATH/bin/go tool dist test --compile-only --rebuild to avoid having to particularly solve #15513. However, it seems that --compile-only does not seem to actually compile the tests? I tried reintroducing the same error you removed in https://golang.org/cl/41752 but --compile-only passes just fine. Maybe I'll just go fix #15513 instead, though, since it would make this trivial.

@bradfitz
Copy link
Contributor Author

bradfitz commented May 2, 2017

Note that dist test's --compile-only is not a promise but just a hint from the builders as an optimization to skip tests if convenient. It would be a bit more work to make sure --compile-only had stronger guarantees.

Alternatively, buildall.bash could just loop over $(go list std cmd) and run go test -c $PKG

@bradfitz
Copy link
Contributor Author

bradfitz commented May 2, 2017

Packages with tests:

$ for PKG in $(go list -f "{{.ImportPath}} {{.TestGoFiles}}" std cmd | \
   grep -v ' \[\]$' | perl -npe 's/ .+//'); do \
       echo $PKG; \
   done

@paranoiacblack
Copy link
Contributor

Until #15513 is resolved, would it be okay to use the go test -run=^$ std cmd workaround? Since ideally, go test -c std cmd should just work anyways.

@bradfitz
Copy link
Contributor Author

bradfitz commented May 2, 2017

@paranoiacblack, no, because buildall.bash always runs on linux-amd64 and go test -run runs the (cross-) compiled binary.

@paranoiacblack paranoiacblack self-assigned this May 3, 2017
@gopherbot
Copy link

CL https://golang.org/cl/42531 mentions this issue.

@gopherbot
Copy link

Change https://golang.org/cl/176439 mentions this issue: cmd/go: run full 'go vet' during 'go test' for packages in GOROOT

@golang golang locked and limited conversation to collaborators May 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

3 participants