RedirectTo matcher

[mapreal19]

Inspired by: https://relishapp.com/rspec/rspec-rails/docs/matchers/redirect-to-matcher

Does it make sense to have this matcher? Currently in my tests:

Expect(response.Header().Get("Location")).To(Equal("/"))

But I was able to use a custom matcher so the code becomes simpler:

Expect(response).To(RedirectTo("/"))

[williammartin]

Hi @mapreal19, can you show me what your custom matcher code looks like? It seems like we'd have to be quite opinionated on the type that we expect i.e. http.Response (or more specifically, things that match an interface with the method Header().

Are there other structures common in the Go ecosystem to represent responses? Do they have Header() methods that look the same?

[mapreal19]

Sure:

package matcher

import (
	"fmt"
	"github.com/onsi/gomega/types"
	"net/http/httptest"
	"reflect"
)

type redirectToMatcher struct {
	expected interface{}
}

func RedirectTo(expected interface{}) types.GomegaMatcher {
	return &redirectToMatcher{
		expected: expected,
	}
}

func (matcher *redirectToMatcher) Match(actual interface{}) (success bool, err error) {
	response, ok := actual.(*httptest.ResponseRecorder)
	if !ok {
		return false, fmt.Errorf("RedirectTo matcher expects an http.Response")
	}

	location := getLocation(response)
	success = reflect.DeepEqual(location, matcher.expected)

	return success, nil
}

func (matcher *redirectToMatcher) FailureMessage(actual interface{}) (message string) {
	return fmt.Sprintf(
		"Expected response to redirect to %#v, recieved: %#v",
		matcher.expected,
		getLocation(actual.(*httptest.ResponseRecorder)),
	)
}

func (matcher *redirectToMatcher) NegatedFailureMessage(actual interface{}) (message string) {
	return fmt.Sprintf(
		"Expected response not to redirect to %#v, recieved: %#v",
		matcher.expected,
		getLocation(actual.(*httptest.ResponseRecorder)),
	)
}

func getLocation(response *httptest.ResponseRecorder) string {
	return response.Header().Get("Location")
}

I'm assuming response is a httptest.ResponseRecorder which I believe is the most common scenario when testing controllers/handlers?

[mapreal19]

@williammartin what if for now we support only httptest.ResponseRecorder? If someone else needs a different type we can make the change later.

[ansd]

@mapreal19 thanks for opening the issue and sending the code.

I have a couple of comments:

1. Inspired by your issue we created an HaveHTTPStatus matcher (see Add HaveHTTPStatus matcher #378).
2. In your code example above, do you think it makes more sense to allow the matcher to be called on both *http.Response and *httptest.ResponseRecorder?
3. I think, we should first call Result() on the *httptest.ResponseRecorder before inspecting the headers since the docs state

To test the headers that were written after a handler completes, use the Result method and see the returned Response value's Header.

4. I'm not sure how much sense it makes to add only a RedirectTo matcher. (Looking at the Rspec Redirect_to matcher, it does way more than a simple string comparison.) When adding a RedirectTo matcher, why not adding matchers for other HTTP response headers such as HaveMIMEType similarly as done in

   gomega/ghttp/handlers.go

   Lines 61 to 67 in 6be6c43

   	//VerifyMimeType returns a handler that verifies that a request has a specified mime type set
   	//in Content-Type header
   	func VerifyMimeType(mimeType string) http.HandlerFunc {
   	return func(w http.ResponseWriter, req *http.Request) {
   	Expect(strings.Split(req.Header.Get("Content-Type"), ";")[0]).Should(Equal(mimeType))
   	}
   	}

   
   I currently don't have a strong opinion whether to add multiple such HTTP response header matchers or none of them. @blgm what do you think?

[mapreal19]

@ansd

For 2. maybe, in all my tests I only used a httptest.ResponseRecorder, so not sure if others will use a http.Response.

For me it was just convenient, it's a minimal implementation. My hope is that others will contribute with more complex edge cases.