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

x/exp/errors/fmt: TestPanics broken #30622

Closed
eliasnaur opened this issue Mar 6, 2019 · 10 comments
Closed

x/exp/errors/fmt: TestPanics broken #30622

eliasnaur opened this issue Mar 6, 2019 · 10 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
Unreleased

Comments

@eliasnaur
Copy link
Contributor

From tip: https://build.golang.org/log/5dd20c71b5e4d2d29ae2230ab47058abb52d88e5

--- FAIL: TestPanics (0.00s)
    fmt_test.go:1684: 1: "%s": got "%!s(PANIC=String method: unexpected EOF)" expected "%!s(PANIC=unexpected EOF)"
    fmt_test.go:1684: 2: "%s": got "%!s(PANIC=String method: 3)" expected "%!s(PANIC=3)"
    fmt_test.go:1684: 4: "%#v": got "%!v(PANIC=GoString method: unexpected EOF)" expected "%!v(PANIC=unexpected EOF)"
    fmt_test.go:1684: 5: "%#v": got "%!v(PANIC=GoString method: 3)" expected "%!v(PANIC=3)"
    fmt_test.go:1684: 6: "%#v": got "[]interface {}{%!v(PANIC=GoString method: 3), %!v(PANIC=GoString method: 3)}" expected "[]interface {}{%!v(PANIC=3), %!v(PANIC=3)}"
    fmt_test.go:1684: 8: "%s": got "%!s(PANIC=Format method: unexpected EOF)" expected "%!s(PANIC=unexpected EOF)"
    fmt_test.go:1684: 9: "%s": got "%!s(PANIC=Format method: 3)" expected "%!s(PANIC=3)"
FAIL
FAIL	golang.org/x/exp/errors/fmt	0.109s
@gopherbot gopherbot added this to the Unreleased milestone Mar 6, 2019
@eliasnaur
Copy link
Contributor Author

@mpvl

@bcmills bcmills added 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. labels Apr 12, 2019
@bcmills
Copy link
Contributor

bcmills commented Jun 28, 2019

This is failure is consistent on the openbsd-arm builder:
https://build.golang.org/log/e345ae0134a58d52d63d97fba243afea814f90a5
https://build.golang.org/log/ab72bb8d8ca9390349ec155239f79df407f83d67
https://build.golang.org/log/e8dc2b6221e15f0facf18685ea458ee8034319f8

@dmitshur dmitshur changed the title x/exp: errors/fmt.TestPanics broken x/exp/errors/fmt: TestPanics broken Sep 17, 2019
@dmitshur
Copy link
Contributor

Issue #32528 has been resolved, and so the tests for the golang.org/x/exp/errors module will be executed when trybots run. That means this test failure will affect all trybot runs in x/exp.

@dmitshur
Copy link
Contributor

I'm not familiar enough with this code to know how the test should be fixed. I can send a CL to skip this test on builders with a reference to this issue. It'd be good to have a fix sooner so that trybots can succeed on x/exp.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/196034 mentions this issue: errors/fmt: disable failing TestPanics test on builders

gopherbot pushed a commit to golang/exp that referenced this issue Sep 18, 2019
@dmitshur @bradfitz
errors/fmt: disable failing TestPanics test on builders
a05e197 
Now that golang/go#32528 has been resolved and inner modules
are being tested too, disable this failing test until it can
be fixed. We don't want to distract people trying to do other
work in x/exp with trybot failures.

Updates golang/go#30622

Change-Id: Ia6f9c2b64100c7a9056a013453caf6e6c76177ad
Reviewed-on: https://go-review.googlesource.com/c/exp/+/196034
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@jba
Copy link
Contributor

jba commented Sep 18, 2019

I think we can remove exp/errors; it's been obsoleted by xerrors and the 1.13 stdlib errors. @mpvl @neild can you confirm?

@bcmills
Copy link
Contributor

bcmills commented Sep 30, 2019

This is still showing up on the android builders, presumably because the GO_BUILDER_NAME environment variable is not being propagated to the subprocess.

Regardless, we shouldn't only hide the failure from the builders: end users may reasonably try to run go test golang.org/x/exp/... on their own machines and expect it to pass.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/204078 mentions this issue: errors/fmt: fix tests for new panic message in Go 1.14

@rsc
Copy link
Contributor

rsc commented Oct 29, 2019

Fixing the tests is trivial. Why not do that? Sent a CL.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/204140 mentions this issue: errors/fmt: re-enable TestPanics test on builders

gopherbot pushed a commit to golang/exp that referenced this issue Oct 30, 2019
@dmitshur
errors/fmt: re-enable TestPanics test on builders
a1ab85d 
The test has been fixed in CL 204078, so there isn't a need
to skip it on builders anymore.

Updates golang/go#30622

Change-Id: I88aeed35f963a9cdfe9b5ee6241956d23c2d23ad
Reviewed-on: https://go-review.googlesource.com/c/exp/+/204140
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Oct 29, 2020
@rsc rsc unassigned mpvl Jun 23, 2022
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
Unreleased
Development

No branches or pull requests
7 participants
@rsc @eliasnaur @dmitshur @bcmills @mpvl @gopherbot @jba