-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: configure TryBots for google.golang.org/protobuf #63597
Comments
Change https://go.dev/cl/540496 mentions this issue: |
The canonical source for the google.golang.org/protobuf module is the go.googlesource.com/protobuf repository. It's also currently being mirrored to GitHub at github.com/protocolbuffers/protobuf-go. Add it as a special project type configured to test on Linux only, as requested in the issue. This can be expanded in the future. For golang/go#63597. Change-Id: I4c82fac2076cd2cd1660728bdf3b2c21be84fb10 Reviewed-on: https://go-review.googlesource.com/c/build/+/540496 Reviewed-by: Heschi Kreinick <heschi@google.com> TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
The trybots (and post-submit builders) have been turned up for this repo in CL 540496. Docs for running trybots is at https://go.dev/wiki/GerritAccess#running-trybots-may-start-trybots. |
Thank you @dmitshur for setting this up! AFAICT, the current configuration is to run "go test -short". That’s a good first signal, but we also have an integration test that’s started by running https://github.com/protocolbuffers/protobuf-go/blob/master/test.bash, which we would like to run automatically. Would that be possible to configure as well? Thanks. |
Change https://go.dev/cl/540278 mentions this issue: |
The protobuf repository (added in CL 540496) is special (i.e., it's not one of the many golang.org/x repos that fits into a project type) and configured to be tested only on Linux. Its owners find it helpful for the longtest run mod to be enabled in presubmit by default. After we finish taking care of the needs of special repositories and see the patterns, we might want to refactor further. For golang/go#63597. Change-Id: I7edcefe8a49f1e89a497af390349412a32936462 Reviewed-on: https://go-review.googlesource.com/c/build/+/540278 Reviewed-by: Heschi Kreinick <heschi@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
That's done. If at some point you think it might make sense to test on more than just Linux, please feel to open a new issue to figure out those details. Thanks. |
Thanks for setting up the longtest. However, I think the longtest might just be doing https://github.com/protocolbuffers/protobuf-go/blob/master/test.bash explicitly runs integration_test.go, which is marked as Can we get a |
Change https://go.dev/cl/541123 mentions this issue: |
Indeed—thank you for catching that, and apologies: I read your comment quickly and failed to realize there was more to getting the integration test started by tesh.bash to run than just configuring longtest builders to always run. I saw that the longtest builder was already passing and thought I just needed to also configure it to run in pre-submit for this repo (CL 540278). I took a look at what it might take to make it work in CL 541123, and the good news are that I think there's a good path forward. It's passing on a Linux machine I tried it on in about 7 minutes with a clean cache, and in about 9 minutes with -race flag, but not yet on our LUCI builders because of a problem with bazel dependency, which I want to discuss. I opened #64066 to track that work, so let's continue the conversation there. Thanks. Closing this issue with the scope limited to turning on basic builders in LUCI. |
Changes to the google.golang.org/protobuf module are currently not automatically tested before submission. It would be great if we could add TryBots for the repo. Testing for linux amd64 should be sufficient.
The text was updated successfully, but these errors were encountered: