Correctly handle assertion failure panics for eventually/consistnetly…

… "g Gomega"s in a goroutine
onsi · Dec 14, 2022 · 78f1660 · 78f1660
1 parent 6ebc0bf
commit 78f1660
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 2 deletions.
diff --git a/go.mod b/go.mod
@@ -5,7 +5,7 @@ go 1.18
 require (
 	github.com/golang/protobuf v1.5.2
 	github.com/google/go-cmp v0.5.9
-	github.com/onsi/ginkgo/v2 v2.5.1
+	github.com/onsi/ginkgo/v2 v2.6.1
 	golang.org/x/net v0.4.0
 	gopkg.in/yaml.v3 v3.0.1
 )

diff --git a/go.sum b/go.sum
@@ -8,6 +8,10 @@ github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/onsi/ginkgo/v2 v2.5.1 h1:auzK7OI497k6x4OvWq+TKAcpcSAlod0doAH72oIN0Jw=
 github.com/onsi/ginkgo/v2 v2.5.1/go.mod h1:63DOGlLAH8+REH8jUGdL3YpCpu7JODesutUjdENfUAc=
+github.com/onsi/ginkgo/v2 v2.6.0 h1:9t9b9vRUbFq3C4qKFCGkVuq/fIHji802N1nrtkh1mNc=
+github.com/onsi/ginkgo/v2 v2.6.0/go.mod h1:63DOGlLAH8+REH8jUGdL3YpCpu7JODesutUjdENfUAc=
+github.com/onsi/ginkgo/v2 v2.6.1 h1:1xQPCjcqYw/J5LchOcp4/2q/jzJFjiAOc25chhnDw+Q=
+github.com/onsi/ginkgo/v2 v2.6.1/go.mod h1:yjiuMwPokqY1XauOgju45q3sJt6VzQ/Fict1LFVcsAo=
 golang.org/x/net v0.4.0 h1:Q5QPcMlvfxFTAPV0+07Xz/MpK9NTXu2VDUuy0FeMfaU=
 golang.org/x/net v0.4.0/go.mod h1:MBQ8lrhLObU/6UmLb4fmbmk5OcyYmqtbGd/9yIeKjEE=
 golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ=

diff --git a/internal/async_assertion.go b/internal/async_assertion.go
@@ -20,6 +20,17 @@ type contextWithAttachProgressReporter interface {
 	AttachProgressReporter(func() string) func()
 }
 
+type asyncGomegaHaltExecutionError struct{}
+
+func (a asyncGomegaHaltExecutionError) GinkgoRecoverShouldIgnoreThisPanic() {}
+func (a asyncGomegaHaltExecutionError) Error() string {
+	return `An assertion has failed in a goroutine.  You should call 
+
+    defer GinkgoRecover()
+
+at the top of the goroutine that caused this panic.  This will allow Ginkgo and Gomega to correctly capture and manage this panic.`
+}
+
 type AsyncAssertionType uint
 
 const (
@@ -229,7 +240,8 @@ func (assertion *AsyncAssertion) buildActualPoller() (func() (interface{}, error
 			}
 			_, file, line, _ := runtime.Caller(skip + 1)
 			assertionFailure = fmt.Errorf("Assertion in callback at %s:%d failed:\n%s", file, line, message)
-			panic("stop execution")
+			// we throw an asyncGomegaHaltExecutionError so that defer GinkgoRecover() can catch this error if the user makes an assertion in a goroutine
+			panic(asyncGomegaHaltExecutionError{})
 		})))
 	}
 	if takesContext {

diff --git a/internal/async_assertion_test.go b/internal/async_assertion_test.go
@@ -696,6 +696,21 @@ var _ = Describe("Asynchronous Assertions", func() {
 					Ω(ig.FailureMessage).Should(BeEmpty())
 				})
 
+				It("correctly handles the case (in concert with Ginkgo) when an assertion fails in a goroutine", func() {
+					count := 0
+					ig.G.Eventually(func(g Gomega) {
+						c := make(chan interface{})
+						go func() {
+							defer GinkgoRecover()
+							defer close(c)
+							count += 1
+							g.Expect(count).To(Equal(3)) //panics!
+						}()
+						<-c
+					}).WithTimeout(30 * time.Millisecond).WithPolling(10 * time.Millisecond).Should(Succeed())
+					Ω(count).Should(Equal(3))
+				})
+
 				Context("when making a ShouldNot assertion", func() {
 					It("doesn't succeed until all extra values are zero, there are no failed assertions, and the matcher is (not) satisfied", func() {
 						counter, s, f, err := 0, "hi", Foo{Bar: "hi"}, errors.New("hi")