smartystreets / goconvey

Go testing in the browser. Integrates with `go test`. Write behavioral tests in Go.
http://smartystreets.github.io/goconvey/
Other
8.27k stars 555 forks source link

Panic when run tests with the latest Go 1.12 version #561

Open idexter opened 5 years ago

idexter commented 5 years ago

I'am trying to run test of grafana 6.0.0. it works fine when I use go 1.11.5, but panics on go 1.12 It seems related with #476 and https://golang.org/doc/go1.12#compiler https://github.com/smartystreets/goconvey/blob/master/convey/gotest/utils.go#L18

dexter@dextermac:~/Projects/src/github.com/grafana/grafana(master)$ go test -v ./pkg/log
=== RUN   TestLogFile

  When logging to file

0 total assertions

--- FAIL: TestLogFile (0.00s)
panic: Convey operation made without context on goroutine stack.
Hint: Perhaps you meant to use `Convey(..., func(c C){...})` ? [recovered]
    panic: Convey operation made without context on goroutine stack.
Hint: Perhaps you meant to use `Convey(..., func(c C){...})` ? [recovered]
    panic: Convey operation made without context on goroutine stack.
Hint: Perhaps you meant to use `Convey(..., func(c C){...})` ?

goroutine 19 [running]:
testing.tRunner.func1(0xc000126100)
    /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:830 +0x388
panic(0x11cd040, 0xc0000a8b70)
    /usr/local/Cellar/go/1.12/libexec/src/runtime/panic.go:522 +0x1b5
github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey.(*context).conveyInner.func2(0xc0000a0360)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey/context.go:232 +0x1c6
panic(0x11cd040, 0xc0000a8b70)
    /usr/local/Cellar/go/1.12/libexec/src/runtime/panic.go:522 +0x1b5
github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey.conveyPanic(...)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey/context.go:20
github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey.mustGetCurrentContext(0xc00005bb98)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey/context.go:52 +0x111
github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey.So(0x11f3800, 0xc0000f9c70, 0x1211898, 0x0, 0x0, 0x0)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey/doc.go:125 +0x22
github.com/grafana/grafana/pkg/log.TestLogFile.func1()
    /Users/dexter/Projects/src/github.com/grafana/grafana/pkg/log/file_test.go:23 +0x61
github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey.parseAction.func1(0x1246da0, 0xc0000a0360)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey/discovery.go:80 +0x24
github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey.(*context).conveyInner(0xc0000a0360, 0x1208868, 0x14, 0xc0000aa940)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey/context.go:261 +0x154
github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey.rootConvey.func1()
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey/context.go:110 +0xf1
github.com/grafana/grafana/vendor/github.com/jtolds/gls.(*ContextManager).SetValues.func1(0x0)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/jtolds/gls/context.go:97 +0x40d
github.com/grafana/grafana/vendor/github.com/jtolds/gls.EnsureGoroutineId.func1()
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/jtolds/gls/gid.go:24 +0x2e
github.com/grafana/grafana/vendor/github.com/jtolds/gls._m(0x0, 0xc0000b2560)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/jtolds/gls/stack_tags.go:74 +0x31
github.com/grafana/grafana/vendor/github.com/jtolds/gls.github_com_jtolds_gls_markS(...)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/jtolds/gls/stack_tags.go:54
github.com/grafana/grafana/vendor/github.com/jtolds/gls.addStackTag(...)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/jtolds/gls/stack_tags.go:49
github.com/grafana/grafana/vendor/github.com/jtolds/gls.EnsureGoroutineId(0xc0000a8ab0)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/jtolds/gls/gid.go:24 +0xd7
github.com/grafana/grafana/vendor/github.com/jtolds/gls.(*ContextManager).SetValues(0xc0000aa910, 0xc0000a8a50, 0xc0000b2520)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/jtolds/gls/context.go:63 +0x14b
github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey.rootConvey(0xc000044f68, 0x3, 0x3)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey/context.go:105 +0x22b
github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey.Convey(0xc000044f68, 0x3, 0x3)
    /Users/dexter/Projects/src/github.com/grafana/grafana/vendor/github.com/smartystreets/goconvey/convey/doc.go:75 +0xbf
github.com/grafana/grafana/pkg/log.TestLogFile(0xc000126100)
    /Users/dexter/Projects/src/github.com/grafana/grafana/pkg/log/file_test.go:21 +0x99
testing.tRunner(0xc000126100, 0x1211768)
    /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:865 +0xc0
created by testing.(*T).Run
    /usr/local/Cellar/go/1.12/libexec/src/testing/testing.go:916 +0x357
FAIL    github.com/grafana/grafana/pkg/log  0.011s
wwwjfy commented 5 years ago

Same issue raised in #559, I believe it's caused by gls, and have raised https://github.com/jtolds/gls/issues/14

wwwjfy commented 5 years ago

Worst case (because it breaks back compatibility), we can use context package and have that in all Convey calls as suggested by @mjl- in #559

haskovec commented 5 years ago

I am seeing the same failure on my projects. Was thinking about moving away from goconvey. Please fix!

WinstonPrivacy commented 5 years ago

Same here. Just upgraded to Go v1.12 and getting this error.

wwwjfy commented 5 years ago

This should already be fixed. Try to update goconvey to latest version.

WinstonPrivacy commented 5 years ago

That was it! We unknowingly had goconvey vendored so it was pointing to an older version.

On Tue, Mar 12, 2019 at 12:46 PM Tony Wang notifications@github.com wrote:

This should already be fixed. Try to update goconvey to latest version.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/smartystreets/goconvey/issues/561#issuecomment-472108791, or mute the thread https://github.com/notifications/unsubscribe-auth/Ai89gU9kzBtXSNAeIBA5UCNgj0g9IQO4ks5vV-fugaJpZM4bWm4G .

mahirk commented 5 years ago

~IGNORE~ The issue was fixed, there was one outdated vendor.

yann0917 commented 5 years ago

same issue

go version
go version go1.12.4 darwin/amd64
yangkghjh commented 5 years ago

fixed by update goconvey

go get github.com/smartystreets/goconvey
SleepyBrett commented 5 years ago

Didn't fix me, here is what did:

Seems like this could have been solved with vendoring.

MXWXZ commented 5 years ago

This code could NOT work:

func TestFoo(t *testing.T) {
    So(1, ShouldEqual, 1)
}

Actually you should do

func TestFoo(t *testing.T) {
    Convey("Foo test", t, func() {
        So(1, ShouldEqual, 1)
    })
}

May help other noobs like me XD

NeoyeElf commented 5 years ago

code contains So(...) in go func like below will trigger this panic

Convey("test", func() {
  var a = 0
  go func(){
    a = 1
    So(a, ShouldEqual, 1)
  }
}

should change to

Convey("test", func() {
  var a = 0
  go func(){
    a = 1
  }
  time.Sleep(time.Millisecond * 10)
  So(a, ShouldEqual, 1)
}

hope this may help

riannucci commented 5 years ago

goconvey can't keep track of the test suites across goroutines automatically; this is not permitted by the way the go language/runtime work.

Take a close read of the documentation for the Convey function: https://godoc.org/github.com/smartystreets/goconvey/convey#Convey

In particular:

Additionally, you may explicitly obtain access to the Convey context by doing:

Convey(description string, action func(c C))

You may need to do this if you want to pass the context through to a goroutine, or to close over the context in a handler to a library which calls your handler in a goroutine (httptest comes to mind).

So, you can do So assertions inside of a goroutine by explicitly using the context object like:

Convey("test", func(c C) {
  var a = 0
  go func(){
    a = 1
    c.So(a, ShouldEqual, 1)
  }
}

Note the c C and c.So. Normally goconvey passes this context implicitly on the stack using gls. When you launch a goroutine, it gets a completely empty, fresh stack, and this context is lost. By explicitly asking and using the convey context, you can still make So assertions (and Convey suites) inside the goroutine. Note that you only need to do this once per stack, so you can go back to using the implicit context once inside a Convey on the goroutine. Example:

Convey("test", func(c C) {
  var a = 0
  go func(){
    a = 1
    c.Convey("inner", func() {
      So(a, ShouldEqual, 1)
    })
  }
}

Hopefully that helps.

songtianyi commented 5 years ago

same issue

go version go1.13 darwin/amd64
panic: Convey operation made without context on goroutine stack.
Hint: Perhaps you meant to use `Convey(..., func(c C){...})` ? [recovered]
    panic: Convey operation made without context on goroutine stack.
Hint: Perhaps you meant to use `Convey(..., func(c C){...})` ? [recovered]
    panic: Convey operation made without context on goroutine stack.
Hint: Perhaps you meant to use `Convey(..., func(c C){...})` ?

i tried to update goconvey and assertions in $GOPATH to the latest version, not work.

rubycut commented 5 years ago

I installed go 1.13.3 with gvm, installed goconvey on top of empty pkg and src dirs and my tests work.

jsongHBO commented 4 years ago

I'm using golang 1.13.10. After updating the github.com/jtolds/gls, it works for me. seems like they've fixed it https://github.com/jtolio/gls/issues/14

leiboo commented 4 years ago

update jtolio/gls will fix this issue