-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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/cmd/coordinator: test packages in nested modules (in multi-module repositories) #32528
Comments
@bcmills Do you know how this was solved in the main go repository, which also has more than one module now? Is it all abstracted away in |
That might not be quite as bad as it seems. If the root module has That said, we should probably have the builders look for |
The modules in the main repo are indeed abstracted away by |
Here's how I expect this can be implemented.
|
Change https://golang.org/cl/194277 mentions this issue: |
Change https://golang.org/cl/194559 mentions this issue: |
It will be necessary to know whether the current environment and configuration has resulted in module mode or GOPATH mode being selected. Do this dynamically by invoking 'go env GOMOD', since the outcome depends on the version of Go and its default GO111MODULE behavior, as well as whether a go.mod file is added to the subrepo, and what environment overrides were applied by coordinator via the BuildConfig.ModulesEnv method. This requires the repository being tested to be available on disk, so move the step of fetching it to happen earlier. For now, this change only detects and logs the build mode. Future changes will use this information further. Updates golang/go#34190 Updates golang/go#32528 Updates golang/go#30233 Change-Id: I641becaaae5b6cfad22961d8864a877241e61cd3 Reviewed-on: https://go-review.googlesource.com/c/build/+/194277 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Alexander Rakoczy <alex@golang.org>
I've tested the coordinator change for this issue on x/build CL 191018 and x/tools CL 191018 by restarting their trybots just now, and watched the build logs very closely. It appears to be working as intended, I'm not seeing any issues. It successfully detected the 5 modules in the x/build repo: And the 2 modules in the x/tools repo, Here are build logs for various builds of the successful trybot run for CL 191018: linux-amd64 (Go tip)
linux-amd64-race (Go tip)
windows-amd64-2016 (Go tip)
linux-amd64 (Go 1.13.x)
linux-amd64 (Go 1.12.x) - tests ran in GOPATH mode, module boundaries ignored
Search the build logs for ":: Running" to find And here's the correctly failing trybot run for CL 182584. It has links to its build logs. |
I initially posted the above comment in the wrong issue at #34247 (comment). I've moved it here, but that issue has some additional discussion. /cc @bcmills |
Filed #34352 as followup. |
Change https://golang.org/cl/196034 mentions this issue: |
Now that golang/go#32528 has been resolved and inner modules are being tested too, disable this failing test until it can be fixed. We don't want to distract people trying to do other work in x/exp with trybot failures. Updates golang/go#30622 Change-Id: Ia6f9c2b64100c7a9056a013453caf6e6c76177ad Reviewed-on: https://go-review.googlesource.com/c/exp/+/196034 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It will be necessary to know whether the current environment and configuration has resulted in module mode or GOPATH mode being selected. Do this dynamically by invoking 'go env GOMOD', since the outcome depends on the version of Go and its default GO111MODULE behavior, as well as whether a go.mod file is added to the subrepo, and what environment overrides were applied by coordinator via the BuildConfig.ModulesEnv method. This requires the repository being tested to be available on disk, so move the step of fetching it to happen earlier. For now, this change only detects and logs the build mode. Future changes will use this information further. Updates golang/go#34190 Updates golang/go#32528 Updates golang/go#30233 Change-Id: I641becaaae5b6cfad22961d8864a877241e61cd3 Reviewed-on: https://go-review.googlesource.com/c/build/+/194277 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Andrew Bonventre <andybons@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Alexander Rakoczy <alex@golang.org>
Previously, we were invoking a single 'go test' run at the repository root with the import path pattern of 'golang.org/x/{repo}/...'. This pattern does not match packages that are located in nested modules in the repository. Look for go.mod files in all subdirectories of the repository to find all inner modules. Then, run 'go test' inside each module root, thus testing all packages in all modules of the repository. If one of the test invocations fails, keep testing others, and report all failures. When looking for inner modules, consider only those that have module path that would not be ignored by the go tool and aren't vendored. This way, go.mod files inside testdata directories aren't treated as if they're proper modules. This is being done only when the tests are running in module mode, since module boundaries don't exist in GOPATH mode. Fixes golang/go#32528 Change-Id: I9f8558982885c9955d3b34127c80c485d713b380 Reviewed-on: https://go-review.googlesource.com/c/build/+/194559 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Right now, the trybots and post-submit builders assume there's only one module in each subrepository, and run
go test [-short] [-race] golang.org/x/subrepo/...
inside the subrepo root directory:https://github.com/golang/build/blob/a3d123a17ca979f921ade4800222d1b8586fff87/cmd/coordinator/coordinator.go#L2443-L2459
That means modules not at the root are not covered.
We have some subrepos that have nested modules (e.g., x/net/h2demo, x/build/maintner/cmd/maintserve), and we may have more in the future. They need to be tested.
/cc @bradfitz @bcmills @ianthehat
Somewhat related to #30233, because the
golang.org/x/subrepo/...
import path pattern matches different packages depending on whether operating in GOPATH mode or module mode.The text was updated successfully, but these errors were encountered: