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

Jump to bottom

x/tools: Tests failing with unified IR builders #52150

Closed
timothy-king opened this issue Apr 4, 2022 · 12 comments
Closed

x/tools: Tests failing with unified IR builders #52150

timothy-king opened this issue Apr 4, 2022 · 12 comments
Assignees
@mdempsky
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Go1.20

Comments

@timothy-king
Copy link
Contributor

Unit tests for golang.org/x/tools/go/internal/gcimporter are failing for linux-amd64-unified builder. Seems to require GOEXPERIMENT=unified. Impacts libraries that use gcimporter too like x/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.

$ GOEXPERIMENT=unified gotip test golang.org/x/tools/go/internal/gcimporter
--- FAIL: TestImportTestdata (0.03s)
    gcimporter_test.go:60: exports.go:11:2: could not import go/ast (object is [go object darwin amd64 devel go1.19-7d1e07049fe Mon Apr 4 17:35:32 2022 +0000 X:regabiwrappers,regabiargs
        ] expected [go object darwin amd64 devel go1.19-7d1e07049fe Mon Apr 4 17:35:32 2022 +0000 X:unified,regabiwrappers,regabiargs
        ])
    gcimporter_test.go:61: go tool compile exports.go failed: exit status 1

Here are all of the failing cases in the log:

% grep FAIL linux-amd64-unified_c370f95c.log.txt
FAIL	golang.org/x/tools/cmd/bundle	0.364s
--- FAIL: TestEndToEnd (2.46s)
--- FAIL: TestConstValueChange (2.30s)
FAIL
FAIL	golang.org/x/tools/cmd/stringer	7.584s
FAIL	golang.org/x/tools/go/analysis/internal/checker	9.389s
FAIL	golang.org/x/tools/go/analysis/internal/facts	0.452s
--- FAIL: TestExitCode (25.01s)
FAIL
FAIL	golang.org/x/tools/go/analysis/multichecker	25.011s
FAIL	golang.org/x/tools/go/gcexportdata	0.022s
--- FAIL: TestImportTestdata (0.10s)
--- FAIL: TestImportStdLib (3.23s)
--- FAIL: TestImportedTypes (0.33s)
--- FAIL: TestImportedConsts (0.00s)
FAIL	golang.org/x/tools/go/internal/gcimporter	3.689s
FAIL	golang.org/x/tools/go/packages	11.584s
FAIL	golang.org/x/tools/go/ssa/ssautil	0.283s
--- FAIL: TestChanges (0.60s)
--- FAIL: TestExportedFields (0.07s)
FAIL	golang.org/x/tools/internal/apidiff	0.672s
FAIL

cc @mdempsky
@timothy-king timothy-king added NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository. labels Apr 4, 2022
@gopherbot gopherbot added this to the Unreleased milestone Apr 4, 2022
@hyangah
Copy link
Contributor

hyangah commented Apr 5, 2022

Did anyone already find the culprit change?
This is blocking all changes made to x/tools.

@findleyr

@hyangah
Copy link
Contributor

hyangah commented Apr 5, 2022

https://go-review.googlesource.com/c/go/+/386004 according to the builder history.

@mdempsky
Copy link
Member

mdempsky commented Apr 5, 2022

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.

@gopherbot
Copy link

Change https://go.dev/cl/398494 mentions this issue: go/internal/gcimporter: import unified IR reader

@gopherbot
Copy link

Change https://go.dev/cl/398615 mentions this issue: all: disable failing tests on linux-amd64-unified builder

@mdempsky
Copy link
Member

mdempsky commented Apr 6, 2022 •
edited

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.

@bcmills bcmills added the Soon This needs to be done soon. (regressions, serious bugs, outages) label Apr 7, 2022
@gopherbot
Copy link

Change https://go.dev/cl/398874 mentions this issue: dashboard: disable tools repo trybot for unified IR builder

gopherbot pushed a commit to golang/build that referenced this issue Apr 7, 2022
@mdempsky
dashboard: disable tools repo trybot for unified IR builder
3e24bb7 
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 <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
@bcmills bcmills modified the milestones: Unreleased, Go1.19 May 4, 2022
@bcmills bcmills removed the Soon This needs to be done soon. (regressions, serious bugs, outages) label May 4, 2022
@bcmills
Copy link
Contributor

bcmills commented May 4, 2022 •
edited

I just noticed that #48595 was filed for what appears to be a related issue back in September. 😅

@gopherbot
Copy link

Change https://go.dev/cl/408699 mentions this issue: dashboard: add known issue for linux-amd64-unified

gopherbot pushed a commit to golang/build that referenced this issue May 26, 2022
@gopherbot
dashboard: add known issue for linux-amd64-unified
e494319 
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 <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
@ianlancetaylor
Copy link
Contributor

@mdempsky Is there anything else to do for this issue? It's currently in the 1.19 milestone. Thanks.

@mdempsky mdempsky modified the milestones: Go1.19, Go1.20 Jul 12, 2022
@dmitshur dmitshur mentioned this issue Aug 8, 2022
Closed
@findleyr
Copy link
Contributor

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:
https://build.golang.org/log/3dc8ca00871869d21258abd40a7b5e0e65a888cb

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?

@gopherbot
Copy link

Change https://go.dev/cl/463156 mentions this issue: dashboard: only test nounified on release-branch.go1.20

@golang golang locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mdempsky mdempsky

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
Release Status & Release Blockers
Status: Done
Milestone
Go1.20
Development

No branches or pull requests
8 participants
@mdempsky @timothy-king @ianlancetaylor @hyangah @bcmills @gopherbot @heschi @findleyr