Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultMatcher isn't strict #97

Closed
fornellas-udemy opened this issue Jun 14, 2024 · 1 comment · Fixed by #98
Closed

DefaultMatcher isn't strict #97

fornellas-udemy opened this issue Jun 14, 2024 · 1 comment · Fixed by #98

Comments

@fornellas-udemy
Copy link
Contributor

fornellas-udemy commented Jun 14, 2024

The provided DefaultMatcher only compares the method and URL of the request. In the general case, there are various conditions where this isn't enough to uniquely identify a request:

  • Two different PUT requests with different bodies. Let's say the server expects a YAML, one has a valid YAML body, the other does not, the cassettes should differentiate both.
  • The request uses headers to define the expected response type, eg Accept: application/json or Accept: application/x-yaml, the cassettes should give back different responses.

While in some particular cases, a permissive matcher may work, in the general case, it is bound to yield inconsistent test signal.

I have written the following FullRequestMatcher matcher below, which compares "the whole request", so there's no chance of a mismatch on the cassette. Arguably, this should replace DefaultMatcher (with a majon release), but there could still be value on adding it alongside DefaultMatcher.

WDYT?

// Similar to reflect.DeepEqual, but considers the contents of collections, so {} and nil would be
// considered equal. works with Array, Map, Slice, or pointer to Array.
func deepEqualContents(x, y any) bool {
	if reflect.ValueOf(x).IsNil() {
		if reflect.ValueOf(y).IsNil() {
			return true
		} else {
			return reflect.ValueOf(y).Len() == 0
		}
	} else {
		if reflect.ValueOf(y).IsNil() {
			return reflect.ValueOf(x).Len() == 0
		} else {
			return reflect.DeepEqual(x, y)
		}
	}
}

func bodyMatches(request *http.Request, cassetteRequest cassette.Request) bool {
	if request.Body != nil {
		var buffer bytes.Buffer

		if _, err := buffer.ReadFrom(request.Body); err != nil {
			panic(fmt.Sprintf("failed to read %s %s body: %s", request.Method, request.URL.String(), err))
		}

		request.Body = io.NopCloser(bytes.NewBuffer(buffer.Bytes()))

		if buffer.String() != cassetteRequest.Body {
			return false
		}
	} else {
		if len(cassetteRequest.Body) != 0 {
			return false
		}
	}

	return true
}

// The DefaultMatcher only cherks for method and URL, which isn't enough to uniquely identify
// requsets. Thus function provides a matcher that matches the whole request, preventing any
// request/response mismatches from happening.
func FullRequestMatcher(request *http.Request, cassetteRequest cassette.Request) bool {
	if request.Method != cassetteRequest.Method {
		return false
	}

	if request.URL.String() != cassetteRequest.URL {
		return false
	}

	if request.ProtoMajor != cassetteRequest.ProtoMajor {
		return false
	}
	if request.ProtoMinor != cassetteRequest.ProtoMinor {
		return false
	}

	if !deepEqualContents(request.Header, cassetteRequest.Headers) {
		return false
	}

	if !bodyMatches(request, cassetteRequest) {
		return false
	}

	if request.ContentLength != cassetteRequest.ContentLength {
		return false
	}

	if !deepEqualContents(request.TransferEncoding, cassetteRequest.TransferEncoding) {
		return false
	}

	if request.Host != cassetteRequest.Host {
		return false
	}

	if !deepEqualContents(request.Trailer, cassetteRequest.Trailer) {
		return false
	}

	if request.RemoteAddr != cassetteRequest.RemoteAddr {
		return false
	}

	if request.RequestURI != cassetteRequest.RequestURI {
		return false
	}

	return true
}
@dnaeon
Copy link
Owner

dnaeon commented Jun 15, 2024

Hey @fornellas-udemy ,

Thanks for the detailed issue.

Could you please submit a PR for this one?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants