-
Notifications
You must be signed in to change notification settings - Fork 18k
x/tools: Tests failing with unified IR builders #52150
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
Comments
Did anyone already find the culprit change? |
https://go-review.googlesource.com/c/go/+/386004 according to the builder history. |
This is because I haven't copied the gcimporter changes to the x/tools repo yet. I knew I needed to do that eventually, but didn't think the x/tools bots were testing against the unified builder. I guess I can prioritize landing that today. |
Change https://go.dev/cl/398494 mentions this issue: |
Change https://go.dev/cl/398615 mentions this issue: |
FYI, I'm out-of-office today (I'll be back tomorrow). I was hoping CL 398615 would be a quick fix I could submit this morning before heading out, but it doesn't look like I'm going to have that ready in time. Each CL revision seems to reveal more failing tests, and the test logs don't always indicate which specific tests within a package actually failed. I'd prefer not to rollback the cmd/compile change, but we can do that if really necessary. If having the "FAIL" text on the dashboard is an issue, I'd lean towards just disabling the unified+x/tools builder temporarily. |
Change https://go.dev/cl/398874 mentions this issue: |
x/tools has a bunch of tests that don't currently work with GOEXPERIMENT=unified. This is a known issue that's going to take some time to fix, so let's just disable the trybot for now to prevent blocking other developers. Updates golang/go#52150. Change-Id: I3fa3ee406a422a4b01cf1ce7753a27be83516f10 Reviewed-on: https://go-review.googlesource.com/c/build/+/398874 Trust: Bryan Mills <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Bryan Mills <[email protected]> Trust: Matthew Dempsky <[email protected]>
I just noticed that #48595 was filed for what appears to be a related issue back in September. 😅 |
Change https://go.dev/cl/408699 mentions this issue: |
For golang/go#52653. Updates golang/go#52150. Change-Id: Ie76101e2a790b77c85a89fe41f4069f8cdd1522d Reviewed-on: https://go-review.googlesource.com/c/build/+/408699 Auto-Submit: Bryan Mills <[email protected]> Run-TryBot: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alex Rakoczy <[email protected]>
@mdempsky Is there anything else to do for this issue? It's currently in the 1.19 milestone. Thanks. |
I believe the major cause of failure has been fixed here, and yet the builder is still grayed out. @mdempsky, can we close this? I'll note that there was an interesting failure on the linux-amd64-unified builder today, that looks like it may perhaps be a real, new bug: Should we close this issue and mark the builders as having no known breakages, so that we will be more likely to notice meaningful unified failures such as the one above? |
Change https://go.dev/cl/463156 mentions this issue: |
Unit tests for
golang.org/x/tools/go/internal/gcimporter
are failing forlinux-amd64-unified
builder. Seems to requireGOEXPERIMENT=unified
. Impacts libraries that use gcimporter too likex/tools/ssa
.Noticed on https://storage.googleapis.com/go-build-log/11ec59a6/linux-amd64-unified_c370f95c.log
Reproduces with gotip @ 5c4ed73 and x/tools @ 4077921.
Here are all of the failing cases in the log:
cc @mdempsky
The text was updated successfully, but these errors were encountered: