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

Ginkgo panics when using the Table extension #234

Closed
robdimsdale opened this issue Mar 14, 2016 · 21 comments
Closed

Ginkgo panics when using the Table extension #234

robdimsdale opened this issue Mar 14, 2016 · 21 comments
Labels

Comments

@robdimsdale
Copy link
Contributor

see https://sunrise.ci.cf-app.com/pipelines/pivnet-resource/jobs/test/builds/135

[2] /tmp/ginkgo371214560/validator.test flag redefined: ginkgo.seed
[2] panic: /tmp/ginkgo371214560/validator.test flag redefined: ginkgo.seed

I see the same thing when locally when not running in parallel.

This is running golang 1.6 and I believe the behavior is not observed on golang 1.5 as I have used the Table extension in other projects on golang 1.5 without issue.

robdimsdale added a commit to pivotal-cf/pivnet-resource that referenced this issue Mar 14, 2016
This reverts commit 95ee3d3.

- It panics locally and in CI. See
  onsi/ginkgo#234
@onsi
Copy link
Owner

onsi commented Mar 14, 2016

very weird... i can't get this to reproduce locally with a minimal example. maybe we can poke at it a bit tomorrow, @robdimsdale ?

@robdimsdale
Copy link
Contributor Author

I ran this with go 1.5.3 and it worked - https://sunrise.ci.cf-app.com/builds/855 is a one-off build demonstrating this behavior (you can't see the SHA but it's the same commit that fails in golang 1.6).

@robdimsdale
Copy link
Contributor Author

pivotal-cf/pivnet-resource@95ee3d3 is the offending commit

@robdimsdale
Copy link
Contributor Author

Stepping away from the specifics of the test for that project and using a minimal example, I can reproduce locally this with the table-driven-testing example given at http://onsi.github.io/ginkgo/#table-driven-tests with the following set of files:

foo.go

package foo

foo_suite_test.go

package foo_test

import (
    . "github.com/onsi/ginkgo"
    . "github.com/onsi/gomega"

    "testing"
)

func TestFoo(t *testing.T) {
    RegisterFailHandler(Fail)
    RunSpecs(t, "Foo Suite")
}

foo_test.go

import (
    . "github.com/onsi/ginkgo/extensions/table"

    . "github.com/onsi/ginkgo"
    . "github.com/onsi/gomega"
)

var _ = Describe("Math", func() {
    DescribeTable("the > inequality",
        func(x int, y int, expected bool) {
            Expect(x > y).To(Equal(expected))
        },
        Entry("x > y", 1, 0, true),
        Entry("x == y", 0, 0, false),
        Entry("x < y", 0, 1, false),
    )
})

and I get the following output:

/var/folders/yq/cqd4_t1n4r1fvn82chvygmh00000gn/T/ginkgo116347267/foo.test flag redefined: ginkgo.seed
panic: /var/folders/yq/cqd4_t1n4r1fvn82chvygmh00000gn/T/ginkgo116347267/foo.test flag redefined: ginkgo.seed

goroutine 1 [running]:
panic(0x35c620, 0xc82000f2e0)
    /usr/local/Cellar/go/1.6/libexec/src/runtime/panic.go:464 +0x3e6
flag.(*FlagSet).Var(0xc82005e060, 0xb04878, 0x6b5560, 0xc82000f290, 0xb, 0x506a40, 0x2a)
    /usr/local/Cellar/go/1.6/libexec/src/flag/flag.go:776 +0x454
flag.(*FlagSet).Int64Var(0xc82005e060, 0x6b5560, 0xc82000f290, 0xb, 0x56e6ccd4, 0x506a40, 0x2a)
    /usr/local/Cellar/go/1.6/libexec/src/flag/flag.go:601 +0x8c
github.com/onsi/ginkgo/config.Flags(0xc82005e060, 0xc820053dd0, 0x7, 0xc820053e01)
    /Users/rdimsdale/go/src/github.com/onsi/ginkgo/config/config.go:64 +0x17a
github.com/onsi/ginkgo.init.1()
    /Users/rdimsdale/go/src/github.com/onsi/ginkgo/ginkgo_dsl.go:54 +0x49
github.com/onsi/ginkgo.init()
    /Users/rdimsdale/go/src/github.com/onsi/ginkgo/ginkgo_dsl.go:558 +0x94
github.com/onsi/ginkgo/extensions/table.init()
    /Users/rdimsdale/go/src/github.com/onsi/ginkgo/extensions/table/table_entry.go:81 +0x45
github.com/pivotal-cf-experimental/pivnet-resource/foo_test.init()
    /Users/rdimsdale/go/src/github.com/pivotal-cf-experimental/pivnet-resource/foo/foo_test.go:19 +0x52
main.init()
    github.com/pivotal-cf-experimental/pivnet-resource/foo/_test/_testmain.go:58 +0x4a

Ginkgo ran 1 suite in 3.125817159s
Test Suite Failed

@robdimsdale
Copy link
Contributor Author

I understand that others have seen this error without using the Table Extension: the cf routing team reported the same panic in cloudfoundry/ginkgo slack channel for the hunk at https://github.com/cloudfoundry/cf-acceptance-tests/blob/wip-108400168-multiports/routing/multiple_app_ports_test.go#L70-L76

From Slack

When I uncomment the hunk in this link, ginkgo barfs when doing DSL stuff with a different test in the package

mark at evilcorp ~/workspace/cf-release/src/github.com/cloudfoundry/cf-acceptance-tests(wip-108400168-multiports ✗) $ ./bin/test routing
++ dirname ./bin/test
+ project_go_root=./bin/../../../../../
+ pushd ./bin/../../../../../
+ project_gopath=/Users/mark/workspace/cf-release
+ popd
+ export GOPATH=/Users/mark/workspace/cf-release:/Users/mark/workspace/go
+ GOPATH=/Users/mark/workspace/cf-release:/Users/mark/workspace/go
+ export PATH=/Users/mark/workspace/cf-release/bin:/Users/mark/.rvm/gems/ruby-2.1.8/bin:/Users/mark/.rvm/gems/ruby-2.1.8@global/bin:/Users/mark/.rvm/rubies/ruby-2.1.8/bin:/Users/mark/.rvm/bin:/Users/mark/bin:/usr/local/bin:/usr/local/sbin:/Users/mark/.vim/bundle/vipe:/Users/mark/workspace/go/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin:/Users/mark/.fzf/bin
+ PATH=/Users/mark/workspace/cf-release/bin:/Users/mark/.rvm/gems/ruby-2.1.8/bin:/Users/mark/.rvm/gems/ruby-2.1.8@global/bin:/Users/mark/.rvm/rubies/ruby-2.1.8/bin:/Users/mark/.rvm/bin:/Users/mark/bin:/usr/local/bin:/usr/local/sbin:/Users/mark/.vim/bundle/vipe:/Users/mark/workspace/go/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin:/Users/mark/.fzf/bin
+ go install -v github.com/cloudfoundry/cf-acceptance-tests/Godeps/_workspace/src/github.com/onsi/ginkgo/ginkgo
+ go list github.com/cloudfoundry/cf-acceptance-tests/...
+ xargs -I '{}' go test -c '{}'
?       github.com/cloudfoundry/cf-acceptance-tests/assets/golang    [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/assets/worker-app    [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/app_helpers    [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/assets    [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/matchers    [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/routing_helpers    [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/services    [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/v3_helpers    [no test files]
+ ginkgo routing
/var/folders/bm/09869zhx6r5fxssbsnn_hfqm0000gn/T/ginkgo558439217/routing.test flag redefined: ginkgo.seed
panic: /var/folders/bm/09869zhx6r5fxssbsnn_hfqm0000gn/T/ginkgo558439217/routing.test flag redefined: ginkgo.seed

goroutine 1 [running]:
flag.(*FlagSet).Var(0xc820018180, 0xc64800, 0x6928e0, 0xc820011ff0, 0xb, 0x4e6960, 0x2a)
    /usr/local/go/src/flag/flag.go:776 +0x46b
flag.(*FlagSet).Int64Var(0xc820018180, 0x6928e0, 0xc820011ff0, 0xb, 0x56de10b7, 0x4e6960, 0x2a)
    /usr/local/go/src/flag/flag.go:601 +0x8c
github.com/onsi/ginkgo/config.Flags(0xc820018180, 0xc8200e3d88, 0x7, 0x3863303334646601)
    /Users/mark/workspace/go/src/github.com/onsi/ginkgo/config/config.go:64 +0x17a
github.com/onsi/ginkgo.init.1()
    /Users/mark/workspace/go/src/github.com/onsi/ginkgo/ginkgo_dsl.go:54 +0x49
github.com/onsi/ginkgo.init()
    /Users/mark/workspace/go/src/github.com/onsi/ginkgo/ginkgo_dsl.go:558 +0x94
github.com/cloudfoundry-incubator/cf-test-helpers/runner.init()
    /Users/mark/workspace/go/src/github.com/cloudfoundry-incubator/cf-test-helpers/runner/run.go:19 +0x56
github.com/cloudfoundry-incubator/cf-test-helpers/cf.init()
    /Users/mark/workspace/go/src/github.com/cloudfoundry-incubator/cf-test-helpers/cf/user_context.go:22 +0x5b
github.com/cloudfoundry/cf-acceptance-tests/routing.init()
    /Users/mark/workspace/cf-release/src/github.com/cloudfoundry/cf-acceptance-tests/routing/session_affinity_test.go:283 +0x6a
main.init()
    github.com/cloudfoundry/cf-acceptance-tests/routing/_test/_testmain.go:56 +0x4a

goroutine 17 [syscall, locked to thread]:
runtime.goexit()
    /usr/local/go/src/runtime/asm_amd64.s:1721 +0x1

goroutine 5 [syscall]:
os/signal.loop()
    /usr/local/go/src/os/signal/signal_unix.go:22 +0x18
created by os/signal.init.1
    /usr/local/go/src/os/signal/signal_unix.go:28 +0x37

Ginkgo ran 1 suite in 1.540324489s
Test Suite Failed

another pair finished that story when I encountered the issue… but I’ll check with them on how they fixed it. From looking at the original remote WIP branch and the current state .. it looks like they avoided the nested context that was originally added (so it might still be an issue, I’ll test by adding another Context)

@onsi
Copy link
Owner

onsi commented Mar 14, 2016

bizzare. i took your example and tried it on my machine running go 1.6 (brew installed) and it works for me... have you cleared your $GOPATH/pkg?

@markstgodard
Copy link

go version go1.5.3 darwin/amd64
ginkgo: (from cf acceptance tests Godeps)

    "ImportPath": "github.com/onsi/ginkgo",
    "Comment": "v1.2.0-44-gac3d45d",
    "Rev": "ac3d45ddd7ef5c4d7fc4d037b615a81f4d67981e"

As Rob mentioned, I did get an error: flag redefined: ginkgo.seed.

I originally got the error when adding a test when working on a routing story... another pair finished the story, however I seemed to be able to narrow down and recreate the problem, If I modify the test as follows:

cloudfoundry/cf-acceptance-tests@108bbc5

Obviously this code is incomplete and not valid... but it should still run... But I get the error: flag redefined: ginkgo.seed

Might be related but something Rob and I discussed.

mark at evilcorp ~/workspace/cf-release/src/github.com/cloudfoundry/cf-acceptance-tests(master ✗) $ bin/test routing
++ dirname bin/test
+ project_go_root=bin/../../../../../
+ pushd bin/../../../../../
+ project_gopath=/Users/mark/workspace/cf-release
+ popd
+ export GOPATH=/Users/mark/workspace/cf-release:/Users/mark/workspace/go
+ GOPATH=/Users/mark/workspace/cf-release:/Users/mark/workspace/go
+ export PATH=/Users/mark/workspace/cf-release/bin:/Users/mark/.rvm/gems/ruby-2.1.8/bin:/Users/mark/.rvm/gems/ruby-2.1.8@global/bin:/Users/mark/.rvm/rubies/ruby-2.1.8/bin:/Users/mark/.rvm/bin:/Users/mark/bin:/usr/local/bin:/usr/local/sbin:/Users/mark/.vim/bundle/vipe:/Users/mark/workspace/go/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin:/Users/mark/.fzf/bin
+ PATH=/Users/mark/workspace/cf-release/bin:/Users/mark/.rvm/gems/ruby-2.1.8/bin:/Users/mark/.rvm/gems/ruby-2.1.8@global/bin:/Users/mark/.rvm/rubies/ruby-2.1.8/bin:/Users/mark/.rvm/bin:/Users/mark/bin:/usr/local/bin:/usr/local/sbin:/Users/mark/.vim/bundle/vipe:/Users/mark/workspace/go/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin:/Users/mark/.fzf/bin
+ go install -v github.com/cloudfoundry/cf-acceptance-tests/Godeps/_workspace/src/github.com/onsi/ginkgo/ginkgo
+ go list github.com/cloudfoundry/cf-acceptance-tests/...
+ xargs -I '{}' go test -c '{}'
?       github.com/cloudfoundry/cf-acceptance-tests/assets/golang   [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/assets/worker-app   [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/app_helpers [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/assets  [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/matchers    [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/services    [no test files]
?       github.com/cloudfoundry/cf-acceptance-tests/helpers/v3_helpers  [no test files]
+ ginkgo routing
/var/folders/bm/09869zhx6r5fxssbsnn_hfqm0000gn/T/ginkgo698077052/routing.test flag redefined: ginkgo.seed
panic: /var/folders/bm/09869zhx6r5fxssbsnn_hfqm0000gn/T/ginkgo698077052/routing.test flag redefined: ginkgo.seed

goroutine 1 [running]:
flag.(*FlagSet).Var(0xc820018180, 0xc64800, 0x6918e0, 0xc8200f4040, 0xb, 0x4e5a00, 0x2a)
    /usr/local/go/src/flag/flag.go:776 +0x46b
flag.(*FlagSet).Int64Var(0xc820018180, 0x6918e0, 0xc8200f4040, 0xb, 0x56e6dd71, 0x4e5a00, 0x2a)
    /usr/local/go/src/flag/flag.go:601 +0x8c
github.com/onsi/ginkgo/config.Flags(0xc820018180, 0xc8200e7d88, 0x7, 0x3863303334646601)
    /Users/mark/workspace/go/src/github.com/onsi/ginkgo/config/config.go:64 +0x17a
github.com/onsi/ginkgo.init.1()
    /Users/mark/workspace/go/src/github.com/onsi/ginkgo/ginkgo_dsl.go:54 +0x49
github.com/onsi/ginkgo.init()
    /Users/mark/workspace/go/src/github.com/onsi/ginkgo/ginkgo_dsl.go:558 +0x94
github.com/cloudfoundry-incubator/cf-test-helpers/runner.init()
    /Users/mark/workspace/go/src/github.com/cloudfoundry-incubator/cf-test-helpers/runner/run.go:19 +0x56
github.com/cloudfoundry-incubator/cf-test-helpers/cf.init()
    /Users/mark/workspace/go/src/github.com/cloudfoundry-incubator/cf-test-helpers/cf/user_context.go:22 +0x5b
github.com/cloudfoundry/cf-acceptance-tests/routing.init()
    /Users/mark/workspace/cf-release/src/github.com/cloudfoundry/cf-acceptance-tests/routing/session_affinity_test.go:281 +0x60
main.init()
    github.com/cloudfoundry/cf-acceptance-tests/routing/_test/_testmain.go:56 +0x4a

goroutine 17 [syscall, locked to thread]:
runtime.goexit()
    /usr/local/go/src/runtime/asm_amd64.s:1721 +0x1

goroutine 5 [syscall]:
os/signal.loop()
    /usr/local/go/src/os/signal/signal_unix.go:22 +0x18
created by os/signal.init.1
    /usr/local/go/src/os/signal/signal_unix.go:28 +0x37

Ginkgo ran 1 suite in 1.652218585s
Test Suite Failed

@robdimsdale
Copy link
Contributor Author

@onsi yes I removed $GOPATH/pkg and $GOPATH/bin and I also tried this on a brand new image (that's in CI above).

However, I have solved my issue. In my case the issue I was seeing was that I had not added github.com/onsi/ginkgo/extensions/table to my vendor directory - once I did this, everything worked fine locally and in CI.

I'm not sure why this manifested itself in the error that I saw, as missing a dependency should be a compile-time error, not a runtime panic due to the same arg being provided twice.

It also doesn't exactly solve the issue @markstgodard was seeing, though I suspect they're related. I wonder if they was missing a vendored copy of the cf-test-helpers dependency?

@markstgodard if you want to dig deeper, I would start by checking that you have the correct version of your dependencies vendored. If you're satisfied, however, we can close this issue.

@markstgodard
Copy link

@robdimsdale I'll check, thx

@markstgodard
Copy link

@robdimsdale I nuked $GOPATH/bin and $GOPATH/pkg and also made sure I did a godep restore from cf-acceptance-tests, however problem still persists for me.
Should be using the version of cf-test-helpers that are in Godeps. . . oddness.

If you wanted to Close, it could be something off in my env, but I'll look deeper.

Cheers

@onsi
Copy link
Owner

onsi commented Mar 15, 2016

Sigh.. Go. The hackery that went into the vendor experiment continues to blindside people (also flags and ginkgo both use global state, so caveat emptor).

OK this makes sense @robdimsdale - if ginkgo is vendored but /table isn't vendored and ginkgo happens to be under your $GOPATH then go will pull in the package under $GOPATH/github.com/onsi/ginkgo/extensions/table which will import the package under $GOPATH/github.com/onsi/ginkgo which runs the init() function and registers the flags globally.

But go will pull in the ginkgo package for your test suite from VENDOR_PATH/github.com/onsi/ginkgo. This will be a different package from the $GOPATH one and go will run it's init() function too (normally the init() function for a package is only run once - but these are treated like two different packages).

The two init()s call the flag package to register ginkgo.seeds and kaboom

@robdimsdale
Copy link
Contributor Author

@onsi makes total sense, thanks for the excellent explanation.

@markstgodard you're using go 1.5.3, correct? Are you setting the GO15VENDOREXPERIMENT environment variable? If not, I wonder how your issue is manifesting. It's probably two separate packages of ginkgo being registered, but which ones? And how?

@glyn
Copy link
Contributor

glyn commented Feb 14, 2017

Note that this problem continues to occur with the official Go vendoring support, e.g. in 1.7.4. The solution is the same: vendor github.com/onsi/ginkgo/extensions/table. So it wasn't just a problem with the vendor experiment.

@glyn
Copy link
Contributor

glyn commented Feb 15, 2017

@onsi @georgeharley (who I was pairing with when we hit this issue) suggested adding a brief note to the docs for tables to make sure people remember to vendor in the extra package. Might be worth it if this is a common problem.

@onsi
Copy link
Owner

onsi commented Feb 15, 2017 via email

@georgeharley
Copy link
Contributor

#322

@rosenhouse
Copy link

rosenhouse commented Mar 27, 2017

We just encountered this problem after we began using code.cloudfoundry.org/lager in our code & tests without first vendoring it into our project's vendor directory. Perhaps it was causing ginkgo to be double imported, and thus adding the flag twice.

To be clear, adding lager to our vendor directory fixed the problem.

@porridge
Copy link

I have a slightly different problem, where adding to my vendor directory does not help.

My project (mything) wants to use both ginkgo and some other library (somedep) which also uses ginkgo and has it vendored:

porridge@kielonek:~/projects/go/src$ find mything/ somedep/ ginkgo/  -type f|xargs head -n 100
==> mything/mything.go <==
package main

import (
        _ "ginkgo"
        _ "somedep"
)
func init() {
        println("main init")
}

func main() {
        println("main")
}

==> somedep/somedep.go <==
package somedep

import (
        _ "ginkgo"
)

func init() {
        println("somedep init")
}

==> somedep/vendor/ginkgo/ginkgo.go <==
package ginkgo

func init() {
        println("ginkgo (vendored) init")
}

==> ginkgo/ginkgo.go <==
package ginkgo

func init() {
        println("ginkgo init")
}

In this case, both copies of ginkgo will be initialized, which leads to the crash in real world:

porridge@kielonek:~/projects/go/src$ go run mything/mything.go 
ginkgo init
ginkgo (vendored) init
somedep init
main init
main
porridge@kielonek:~/projects/go/src$ 

I'm rather new to golang, and the only solution I can come up with is:

  1. copy somedep to mything/vendor/ and then
  2. rip out mything/vendor/ginkgo.

This of course seems far from elegant. Does anyone have any better ideas?

Is there perhaps anything that can be done to ginkgo to just issue a warning and not crash in this scenario?

@carldunham
Copy link
Contributor

I am seeing a version of this as well, related to the way that Go mis-handles vendored packages generally (hopefully fixed with module support in 1.11), as illustrated in https://github.com/carldunham/go-dep-test.

It would be helpful if we could set the prefix variable for the flags, at least, so we could hack something up where each instance of Ginkgo could be assigned a different prefix. Apologies if this is already a thing, and I just overlooked it.

@carldunham
Copy link
Contributor

I submitted a PR to suggest one way of addressing this: #502

@onsi
Copy link
Owner

onsi commented Apr 6, 2021

I'm working through the backlog of old Ginkgo issues - apologies as this issue is probably stale now.
I'm going to close this for now, but feel free to reopen it if you'd like.

@onsi onsi closed this as completed Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants