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.

Sign up for GitHub

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

cmd/compile/internal/noder: TestUnifiedCompare fails with changes in dev.fuzz branch #48265

Closed
jayconrod opened this issue Sep 8, 2021 · 3 comments
Closed

cmd/compile/internal/noder: TestUnifiedCompare fails with changes in dev.fuzz branch #48265

jayconrod opened this issue Sep 8, 2021 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.19

Comments

@jayconrod
Copy link
Contributor

I noticed this when merging master into dev.fuzz, originally at CL 347232 PS1. See test log. The error is:

--- FAIL: TestUnifiedCompare (29.96s)
    --- FAIL: TestUnifiedCompare/linux/amd64 (29.95s)
        unified_test.go:121: running /workdir/go/bin/go list -e -export -json -gcflags=all=-d=unified=0 -d=inlfuncswithclosures=0 -d=unifiedquirks=1 -G=0 -- std
        unified_test.go:121: running /workdir/go/bin/go list -e -export -json -gcflags=all=-d=unified=1 -d=inlfuncswithclosures=0 -d=unifiedquirks=1 -G=0 -- std
        unified_test.go:95: package "testing/internal/testdeps": compile output differs
FAIL
FAIL	cmd/compile/internal/noder	29.971s

We use type aliases in testing/internal/testdeps to avoid an import cycle between testing and internal tests of internal/fuzz and the packages it depends on. That may be tripping something here.

Per discussion with @mdempsky, it sounds like this may be an overly strict test. We'll skip it on dev.fuzz for now, but we won't merge dev.fuzz into master before the test is fixed.
@jayconrod jayconrod added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. labels Sep 8, 2021
@jayconrod jayconrod added this to the Go1.18 milestone Sep 8, 2021
@ianlancetaylor
Copy link
Member

@mdempsky This is in the 1.18 milestone; time to move to 1.19? Thanks.

As far as I can tell the test doesn't pass right now, but it doesn't seem to be due to fuzzing.

=== RUN   TestUnifiedCompare/linux/amd64
    unified_test.go:128: running /home/iant/go/bin/go list -e -export -json -gcflags=all=-d=unified=0 -d=inlfuncswithclosures=0 -d=unifiedquirks=1 -G=0 -- std
# constraints
../../../../constraints/constraints.go:13:2: syntax error: unexpected ~, expecting method or interface name
../../../../constraints/constraints.go:20:2: syntax error: unexpected ~, expecting method or interface name
../../../../constraints/constraints.go:27:9: syntax error: unexpected |, expecting semicolon or newline or }
../../../../constraints/constraints.go:34:2: syntax error: unexpected ~, expecting method or interface name
../../../../constraints/constraints.go:41:2: syntax error: unexpected ~, expecting method or interface name
../../../../constraints/constraints.go:49:10: syntax error: unexpected |, expecting semicolon or newline or }
    unified_test.go:149: exit status 2

@mdempsky mdempsky modified the milestones: Go1.18, Go1.19 Jan 29, 2022
@mdempsky
Copy link
Contributor

That failure looks like it's because std now includes package constraints, which can't be compiled with -G=0 (because it relies on generics).

I'm leaning towards just removing TestUnifiedCompare anyway. I feel like it's served its purpose of bootstrapping unified IR, and at this point preserving byte-for-byte identical output with -G=0 seems to slow down development more than help.

@cherrymui
Copy link
Member

Both the dev.fuzz branch and the constraint package are gone. I think we can close this.

@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.19
Development

No branches or pull requests
5 participants
@mdempsky @jayconrod @ianlancetaylor @gopherbot @cherrymui