ghttp handles failures and panics in handlers
- If a registered handler makes a failing assertion ghttp will return 500.
- If a registered handler panics, ghttp will return 500 *and* fail the test. This is new behavior that may cause existing code to break. This code is almost certainly incorrect and creating a false positive.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5f2c1ed..0eec5a8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,9 @@
- Added `HavePrefix` and `HaveSuffix` matchers.
- `ghttp` can now handle concurrent requests.
- Added `Succeed` which allows one to write `Ω(MyFunction()).Should(Succeed())`.
+- Improved `ghttp`'s behavior around failing assertions and panics:
+ - If a registered handler makes a failing assertion `ghttp` will return `500`.
+ - If a registered handler panics, `ghttp` will return `500` *and* fail the test. This is new behavior that may cause existing code to break. This code is almost certainly incorrect and creating a false positive.
Bug Fixes:
- gexec: `session.Wait` now uses `EventuallyWithOffset` to get the right line number in the failure.
diff --git a/ghttp/test_server.go b/ghttp/test_server.go
index 4bbb2e4..7b4a996 100644
--- a/ghttp/test_server.go
+++ b/ghttp/test_server.go
@@ -111,6 +111,7 @@
"net/http/httptest"
"reflect"
"regexp"
+ "strings"
"sync"
. "github.com/onsi/gomega"
@@ -208,7 +209,29 @@
func (s *Server) ServeHTTP(w http.ResponseWriter, req *http.Request) {
s.writeLock.Lock()
defer func() {
- recover()
+ e := recover()
+ if e != nil {
+ w.WriteHeader(http.StatusInternalServerError)
+ }
+
+ //If the handler panics GHTTP will silently succeed. This is bad™.
+ //To catch this case we need to fail the test if the handler has panicked.
+ //However, if the handler is panicking because Ginkgo's causing it to panic (i.e. an asswertion failed)
+ //then we shouldn't double-report the error as this will confuse people.
+
+ //So: step 1, if this is a Ginkgo panic - do nothing, Ginkgo's aware of the failure
+ eAsString, ok := e.(string)
+ if ok && strings.Contains(eAsString, "defer GinkgoRecover()") {
+ return
+ }
+
+ //If we're here, we have to do step 2: assert that the error is nil. This assertion will
+ //allow us to fail the test suite (note: we can't call Fail since Gomega is not allowed to import Ginkgo).
+ //Since a failed assertion throws a panic, and we are likely in a goroutine, we need to defer within our defer!
+ defer func() {
+ recover()
+ }()
+ Ω(e).Should(BeNil(), "Handler Panicked")
}()
s.receivedRequests = append(s.receivedRequests, req)
diff --git a/ghttp/test_server_test.go b/ghttp/test_server_test.go
index 82aa3aa..25260b6 100644
--- a/ghttp/test_server_test.go
+++ b/ghttp/test_server_test.go
@@ -192,6 +192,51 @@
})
})
+ Describe("When a handler fails", func() {
+ BeforeEach(func() {
+ s.UnhandledRequestStatusCode = http.StatusForbidden //just to be clear that 500s aren't coming from unhandled requests
+ })
+
+ Context("because the handler has panicked", func() {
+ BeforeEach(func() {
+ s.AppendHandlers(func(w http.ResponseWriter, req *http.Request) {
+ panic("bam")
+ })
+ })
+
+ It("should respond with a 500 and make a failing assertion", func() {
+ var resp *http.Response
+ var err error
+
+ failures := InterceptGomegaFailures(func() {
+ resp, err = http.Get(s.URL())
+ })
+
+ Ω(err).ShouldNot(HaveOccurred())
+ Ω(resp.StatusCode).Should(Equal(http.StatusInternalServerError))
+ Ω(failures).Should(ConsistOf(ContainSubstring("Handler Panicked")))
+ })
+ })
+
+ Context("because an assertion has failed", func() {
+ BeforeEach(func() {
+ s.AppendHandlers(func(w http.ResponseWriter, req *http.Request) {
+ // Ω(true).Should(BeFalse()) <-- would be nice to do it this way, but the test just can't be written this way
+
+ By("We're cheating a bit here -- we're throwing a GINKGO_PANIC which simulates a failed assertion")
+ panic(GINKGO_PANIC)
+ })
+ })
+
+ It("should respond with a 500 and *not* make a failing assertion, instead relying on Ginkgo to have already been notified of the error", func() {
+ resp, err := http.Get(s.URL())
+
+ Ω(err).ShouldNot(HaveOccurred())
+ Ω(resp.StatusCode).Should(Equal(http.StatusInternalServerError))
+ })
+ })
+ })
+
Describe("Request Handlers", func() {
Describe("VerifyRequest", func() {
BeforeEach(func() {
@@ -310,7 +355,7 @@
failures := InterceptGomegaFailures(func() {
http.DefaultClient.Do(req)
})
- Ω(failures).Should(HaveLen(1))
+ Ω(failures).Should(ContainElement(ContainSubstring("Authorization header must be specified")))
})
})
