From 7256da5fe7a8074acf432546a884d1b32faf304d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maxime=20Soul=C3=A9?= Date: Tue, 21 Dec 2021 15:15:40 +0100 Subject: [PATCH] feat: NewNotFoundResponder() helps to detect a possible method mistake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maxime Soulé --- internal/stack_tracer.go | 12 +++++++++--- internal/stack_tracer_test.go | 4 ++++ response.go | 20 +++++++++++++++----- transport.go | 30 +++++++++++++++++++++++++++++- transport_test.go | 29 ++++++++++++++++++++++++----- 5 files changed, 81 insertions(+), 14 deletions(-) diff --git a/internal/stack_tracer.go b/internal/stack_tracer.go index c771983..7072da0 100644 --- a/internal/stack_tracer.go +++ b/internal/stack_tracer.go @@ -13,11 +13,17 @@ type StackTracer struct { Err error } -func (n StackTracer) Error() string { - if n.Err == nil { +// Error implements error interface. +func (s StackTracer) Error() string { + if s.Err == nil { return "" } - return n.Err.Error() + return s.Err.Error() +} + +// Unwrap implements the interface needed by errors.Unwrap. +func (s StackTracer) Unwrap() error { + return s.Err } // CheckStackTracer checks for specific error returned by diff --git a/internal/stack_tracer_test.go b/internal/stack_tracer_test.go index 9ec9a1a..0a7e083 100644 --- a/internal/stack_tracer_test.go +++ b/internal/stack_tracer_test.go @@ -21,6 +21,10 @@ func TestStackTracer(t *testing.T) { if st.Error() != "foo" { t.Errorf("Error() returned <%s> instead of ", st.Error()) } + + if werr := st.Unwrap(); werr != st.Err { + t.Errorf("Unwrap() returned <%s> instead of ", werr) + } } func TestCheckStackTracer(t *testing.T) { diff --git a/response.go b/response.go index 7cef24c..9816b19 100644 --- a/response.go +++ b/response.go @@ -18,6 +18,16 @@ import ( "github.com/jarcoal/httpmock/internal" ) +// fromThenKeyType is used by Then(). +type fromThenKeyType struct{} + +var fromThenKey = fromThenKeyType{} + +// suggestedMethodKeyType is used by NewNotFoundResponder(). +type suggestedMethodKeyType struct{} + +var suggestedMethodKey = suggestedMethodKeyType{} + // Responder is a callback that receives an http request and returns // a mocked response. type Responder func(*http.Request) (*http.Response, error) @@ -125,10 +135,6 @@ func (r Responder) Delay(d time.Duration) Responder { } } -type fromThenKeyType struct{} - -var fromThenKey = fromThenKeyType{} - var errThenDone = errors.New("ThenDone") // similar is simple but a bit tricky. Here we consider two Responder @@ -330,9 +336,13 @@ func NewErrorResponder(err error) Responder { // at /go/src/runtime/asm_amd64.s:1337 func NewNotFoundResponder(fn func(...interface{})) Responder { return func(req *http.Request) (*http.Response, error) { + suggestedMethod, _ := req.Context().Value(suggestedMethodKey).(string) + if suggestedMethod != "" { + suggestedMethod = ", but one matches method " + suggestedMethod + } return nil, internal.StackTracer{ CustomFn: fn, - Err: fmt.Errorf("Responder not found for %s %s", req.Method, req.URL), + Err: fmt.Errorf("Responder not found for %s %s%s", req.Method, req.URL, suggestedMethod), } } } diff --git a/transport.go b/transport.go index 02ff60a..4523f0a 100644 --- a/transport.go +++ b/transport.go @@ -2,6 +2,7 @@ package httpmock import ( "bytes" + "context" "errors" "fmt" "net/http" @@ -208,6 +209,10 @@ func (m *MockTransport) RoundTrip(req *http.Request) (*http.Response, error) { // we didn't find a responder, so fire the 'no responder' responder m.callCountInfo[internal.NoResponder]++ m.totalCallCount++ + // give a hint to NewNotFoundResponder() if it is a possible method error + if suggestedMethod != "" { + req = req.WithContext(context.WithValue(req.Context(), suggestedMethodKey, suggestedMethod)) + } responder = m.noResponder } m.mu.Unlock() @@ -552,7 +557,30 @@ func sortedQuery(m url.Values) string { } // RegisterNoResponder is used to register a responder that is called -// if no other responder is found. The default is httpmock.ConnectionFailure. +// if no other responder is found. The default is httpmock.ConnectionFailure +// that returns an error able to indicate a possible method mismatch. +// +// Use it in conjunction with NewNotFoundResponder to ensure that all +// routes have been mocked: +// +// import ( +// "testing" +// "github.com/jarcoal/httpmock" +// ) +// ... +// func TestMyApp(t *testing.T) { +// ... +// // Calls testing.Fatal with the name of Responder-less route and +// // the stack trace of the call. +// httpmock.RegisterNoResponder(httpmock.NewNotFoundResponder(t.Fatal)) +// +// Will abort the current test and print something like: +// transport_test.go:735: Called from net/http.Get() +// at /go/src/github.com/jarcoal/httpmock/transport_test.go:714 +// github.com/jarcoal/httpmock.TestCheckStackTracer() +// at /go/src/testing/testing.go:865 +// testing.tRunner() +// at /go/src/runtime/asm_amd64.s:1337 func (m *MockTransport) RegisterNoResponder(responder Responder) { m.mu.Lock() m.noResponder = responder diff --git a/transport_test.go b/transport_test.go index 7886642..dce9b65 100644 --- a/transport_test.go +++ b/transport_test.go @@ -46,7 +46,7 @@ func TestMockTransport(t *testing.T) { if err == nil { t.Fatal("An error should occur") } - if !strings.Contains(err.Error(), NoResponderFound.Error()) { + if !strings.HasSuffix(err.Error(), NoResponderFound.Error()) { t.Fatal(err) } @@ -60,7 +60,7 @@ func TestMockTransport(t *testing.T) { if err == nil { t.Fatal("An error should occur") } - if !strings.Contains(err.Error(), + if !strings.HasSuffix(err.Error(), NoResponderFound.Error()+" for method Get, but one matches method GET") { t.Fatal(err) } @@ -74,7 +74,7 @@ func TestMockTransport(t *testing.T) { if err == nil { t.Fatal("An error should occur") } - if !strings.Contains(err.Error(), + if !strings.HasSuffix(err.Error(), NoResponderFound.Error()+" for method POST, but one matches method GET") { t.Fatal(err) } @@ -88,7 +88,7 @@ func TestMockTransport(t *testing.T) { if err == nil { t.Fatal("An error should occur") } - if !strings.Contains(err.Error(), + if !strings.HasSuffix(err.Error(), NoResponderFound.Error()+" for method POST, but one matches method GET") { t.Fatal(err) } @@ -182,8 +182,27 @@ func TestMockTransportNoResponder(t *testing.T) { if err != nil { t.Fatal("expected request to succeed") } - assertBody(t, resp, "hello world") + + // Using NewNotFoundResponder() + RegisterNoResponder(NewNotFoundResponder(nil)) + _, err = http.Get(testURL) + if err == nil { + t.Fatal("an error should occur") + } + if !strings.HasSuffix(err.Error(), "Responder not found for GET http://www.example.com/") { + t.Fatalf("Unexpected error content: %s", err) + } + + // Help the user in case a Responder exists for another method + RegisterResponder("POST", testURL, NewStringResponder(200, "hello world")) + _, err = http.Get(testURL) + if err == nil { + t.Fatal("an error should occur") + } + if !strings.HasSuffix(err.Error(), "Responder not found for GET http://www.example.com/, but one matches method POST") { + t.Fatalf("Unexpected error content: %s", err) + } } func TestMockTransportQuerystringFallback(t *testing.T) {