when using concurrency, a test case failure in a background go routine does not show which test case failed about ginkgo HOT 3 CLOSED

hey @plastikfan - unfortunately, in Go, there is no way for one goroutine to catch a panic thrown in a different goroutine. So there's nothing Ginkgo can do to catch this panic and associate it with a test. Even worse: the panic causes the entire suite to exit immediately and a bunch of tests aren't being run. This is one of many reasons why Go nudges you towards handling issues by returning errors instead of panicking.

if you are launching a goroutine that might panic or that is making explicit assertions (e.g. calling Fail or using a Gomega matcher) then you should put defer GinkgoRecover() at the top of your goroutine. This gives Ginkgo an explicit hook to catch the panic and associate it with the current spec:

func doStuff(c chan bool) {
    defer GinkgoRecover()
    panic("welp")
    close(c)
}

It("works", func() {
    c := make(chan bool)
    go doStuff(c)
    Eventually(c).Should(BeClosed())
})

this will work correctly.

If you can instrument the goroutines in this way then that'll work. If not (e.g. they're in your production code) you'll need to switch to a different way of handling errors. If, on the other hand, they are in a 3rd party library that you've imported then you should open an issue - in general production code should not panic in a goroutine in this way as it could bring production services down.

from ginkgo.

Is that frowned upon too?

I imagine opinions will vary. And I get what you’re saying. There are times when you want to fail fast. regexp.MustCompile() is an example where the standard library will panic and blow up. The intent is to give the developer a fast feedback loop when they are building a (hard coded) regular expression. Other examples are panicking when starting a service up if it fails to (for example) read its configuration, or open a port - in such cases the service is about to exit anyway so might as well panic.

Throwing a panic in a library is doable if you have a similar “you’re using it wrong” use case. But doing this in a goroutine makes it (a) hard for you to test the edge-case (not just in the context of Ginkgo, any go-based test suite will struggle since you can’t intercept the panic in another goroutine) and (b) hard for the user to catch and handle the panic (e.g. “I know I’m using this wrong in this case but rather than fix it I’m happy to just catch the error and try something else” <= this becomes impossible if the panic is in a bit of code that’s typically running in a separate goroutine from the original caller/consumer.).

from ginkgo.

Hi @onsi, thank you for your quick reply. And that is great advice, I'll give it a shot. The panic is not caused by an explicit panic, its caused by an accidental nil pointer de-reference, but I'll definitely take heed of your adice about preferring errors rather than raising a panic. The only exception I have to this rule, particluarly when creating a library, is when a particular issue is known to be a programming error/misuse, in which case, I like to fail loudly with an informative error message. Is that frowned upon too?

from ginkgo.