-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: add a darwin/amd64 longtest builder #35678
Comments
(This message brought to you by Lines 49 to 53 in a23f9af
|
This was moved to the I'd still like to get rid of the builder check in the More clarity about when this is likely to happen would help me evaluate the tradeoffs of that change. |
Change https://go.dev/cl/474137 mentions this issue: |
Also annotate calls to tooSlow with specific reasons. This will somewhat reduce test coverage on the 'darwin' builders until we have darwin 'longtest' builders (#35678,#49055), but still seems worthwhile to avoid alert fatigue from tests that really shouldn't be running in the short configurations. Fixes #58918. Fixes #58919. Change-Id: I0000f0084b262beeec3eca3e9b8a45d61fab4313 Reviewed-on: https://go-review.googlesource.com/c/go/+/474137 Reviewed-by: Ian Lance Taylor <[email protected]> Auto-Submit: Bryan Mills <[email protected]> Run-TryBot: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
Change https://go.dev/cl/474580 mentions this issue: |
Change https://go.dev/cl/474581 mentions this issue: |
…est builders Also annotate calls to tooSlow with specific reasons. This will somewhat reduce test coverage on the 'darwin' builders until we have darwin 'longtest' builders (#35678,#49055), but still seems worthwhile to avoid alert fatigue from tests that really shouldn't be running in the short configurations. Updates #58918. Updates #58919. Fixes #58937. Change-Id: I0000f0084b262beeec3eca3e9b8a45d61fab4313 Reviewed-on: https://go-review.googlesource.com/c/go/+/474137 Reviewed-by: Ian Lance Taylor <[email protected]> Auto-Submit: Bryan Mills <[email protected]> Run-TryBot: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> (cherry picked from commit 9f532dd) Reviewed-on: https://go-review.googlesource.com/c/go/+/474581
…est builders Also annotate calls to tooSlow with specific reasons. This will somewhat reduce test coverage on the 'darwin' builders until we have darwin 'longtest' builders (#35678,#49055), but still seems worthwhile to avoid alert fatigue from tests that really shouldn't be running in the short configurations. Updates #58918. Updates #58919. Fixes #58938. Change-Id: I0000f0084b262beeec3eca3e9b8a45d61fab4313 Reviewed-on: https://go-review.googlesource.com/c/go/+/474137 Reviewed-by: Ian Lance Taylor <[email protected]> Auto-Submit: Bryan Mills <[email protected]> Run-TryBot: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> (cherry picked from commit 9f532dd) Reviewed-on: https://go-review.googlesource.com/c/go/+/474580
It's still in the backlog, no particular ETA. Adding the builder obviously isn't too hard, but we'd have to add more machines and that means re-evaluating our budget. |
FWIW, from my perspective the builder would be useful even if it doesn't have enough machines to run at every CL. (Many of the reverse builders are in that state, and they're still useful for detecting deterministic failures, narrowing down bisection windows, and performing |
Change https://go.dev/cl/479837 mentions this issue: |
(I don't know why it isn't in the logged environment, though. 🤷♂️) |
Thanks, I found that but misread some conditions.
I think this is because we are only logging the environment when running make.bash. For tests we add GOPROXY here, only after running make.bash. |
Change https://go.dev/cl/482335 mentions this issue: |
This field was added in CL 166218 but never used. For golang/go#35678. Change-Id: I197fd3835136e7649b20a7ca47d4179591cb49a5 Reviewed-on: https://go-review.googlesource.com/c/build/+/482335 Auto-Submit: Michael Pratt <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Run-TryBot: Michael Pratt <[email protected]>
Change https://go.dev/cl/482339 mentions this issue: |
Change https://go.dev/cl/482915 mentions this issue: |
The existing behavior for setting GOPROXY is rather hard to follow, and doesn't work correctly in many cases. For example, longtest on a reverse builder gets the GKE proxy. Before CL 479837 there were no longtest builders outside of GCE, so this case was never covered. Fixing this is the motivation of this CL. They way configuration works today is: 1. buildstatus.go unconditionally sets GOPROXY to the GKE proxy [1]. 2. st.conf.ModuleEnv potentially overrides GOPROXY with a more reasonable setting, with a bunch of complex conditions. Unify and simplify this process by moving it into buildstatus.go, where their is now a strict ordering of possible GOPROXY values. Notable changes: * The GKE proxy is never used outside of GCE. * There is a consistent default/fallback of proxy.golang.org. I initially tried to split this into two CLs: one unifying the implementation and the next changing the behavior, but the old behavior is so mind-boggling that the first CL doesn't really make much sense. The annoying part of this CL is that tests move from dashboard to cmd/coordinator, requiring us to export additional fields so cmd/coordinator tests can configure the builders. The test cases themselves are unchanged except for the addition of a non-GCE longtest builder case. [1] Except in runSubrepoTests, which avoids doing so for reverse builders. This was a workaround for private proxy builders in CL 275412, but wasn't extended to other callers because only subrepo tests were seeing a regression. More strangeness. For golang/go#35678. Change-Id: I6090c8c5e91ce6be9bfc07c16f36ed339c9d27ae Reviewed-on: https://go-review.googlesource.com/c/build/+/482339 Reviewed-by: Bryan Mills <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Carlos Amedee <[email protected]> Run-TryBot: Michael Pratt <[email protected]>
For golang/go#35678. Change-Id: I1d408c6547e689702311d73b5bf56adf4d1cd150 Reviewed-on: https://go-review.googlesource.com/c/build/+/482915 Run-TryBot: Michael Pratt <[email protected]> Reviewed-by: Carlos Amedee <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
The latest failures are in crypto/x509: https://build.golang.org/log/8815b334c89791bc3c30410757d4c1020e927e96
@rolandshoemaker any idea what is going on here? Do the builders need some certificate verification system dependency installed/enabled? |
FWIW, that's #57428. |
Ah, nice! So that test needs a skip for now it seems. |
Change https://go.dev/cl/482165 mentions this issue: |
For #57428. For #35678. Change-Id: I806c16d3ff3815b8681916753338356c444970d2 Reviewed-on: https://go-review.googlesource.com/c/go/+/482165 Reviewed-by: Bryan Mills <[email protected]> Auto-Submit: Michael Pratt <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Michael Pratt <[email protected]>
Change https://go.dev/cl/484216 mentions this issue: |
The list_goroot_symlink test relies on fsys.Walk (and ultimately syscall.Lstat) conforming to POSIX pathname resolution semantics. POSIX requires that symlinks ending in a slash be fully resolved, but it appears that lstat in current darwin kernels does not fully resolve the last pathname component when it is a symlink to a symlink. For #59586. For #35678. Change-Id: I37526f012ba94fa1796b33109a41c3226c967d3e Reviewed-on: https://go-review.googlesource.com/c/go/+/484216 Run-TryBot: Bryan Mills <[email protected]> Reviewed-by: Michael Pratt <[email protected]> Auto-Submit: Bryan Mills <[email protected]> TryBot-Bypass: Bryan Mills <[email protected]>
Change https://go.dev/cl/484295 mentions this issue: |
On macOS, TMPDIR is typically a symlink, and the GOROOT for the buildlet is in TMPDIR as well. PWD must be preserved in order for os.Getwd (and functions based on it) to report paths that remain relative to GOROOT, and paths relative to GOROOT are necessary in order for filepath.Rel to report subdirectories as subdirectories (rather than paths with long "../../…" prefixes). Fortunately, the (*Cmd).Environ method added for #50599 makes preserving PWD somewhat easier. This fixes 'go test cmd/internal/moddeps' on the new darwin-amd64-longtest builder. For #35678. Change-Id: Ibaa458bc9a94b44ba455519bb8da445af07fe0d2 Reviewed-on: https://go-review.googlesource.com/c/go/+/484295 Auto-Submit: Bryan Mills <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Run-TryBot: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Michael Pratt <[email protected]>
Change https://go.dev/cl/484476 mentions this issue: |
Just mentioning that this post-submit builder is failing in the 1.19 and 1.20 release branches. |
Change https://go.dev/cl/492055 mentions this issue: |
The darwin longtest builder was added during the Go 1.21 dev cycle and needed some development work before it would pass all tests. Disable it on older release branches that don't have said work, and make it run on Go 1.21 and newer release branches as a TryBot like all other longtest builders. This makes it consistent with other -longtest builders and will help avoid entering broken state when backporting to future release branches. Also remove redundant version check from windows longtest tryBot policy functions since the version check is already there in buildsRepo policy. For golang/go#35678. For golang/go#37827. Change-Id: I09c479f1be2b3635c385b504b0c86b50e65e696b Reviewed-on: https://go-review.googlesource.com/c/build/+/492055 TryBot-Result: Gopher Robot <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Michael Pratt <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]>
Change https://go.dev/cl/503758 mentions this issue: |
A longtest builder for the darwin/amd64 port was added in the Go 1.21 development cycle. Start using it as for non-advisory release testing. For golang/go#40561. For golang/go#35678. For golang/go#29252. Change-Id: Ic65e84e5e10bcb786cb28c36c1d1b137e2f6202e Reviewed-on: https://go-review.googlesource.com/c/build/+/503758 Run-TryBot: Dmitri Shuralyov <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]> Auto-Submit: Dmitri Shuralyov <[email protected]>
Now that we have a Windows longtest builder (#26529), I'm wondering where else we might have a coverage gap.
It would be nice to have a
darwin
builder running non-short tests too. I doubt that it will turn up many regressions forcmd/go
in particular, but the system calls are enough different — and the installed base ofcmd/go
users onmacOS
is large enough — that I suspect it would have a net positive value.CC @golang/release
The text was updated successfully, but these errors were encountered: