Add Unit Tests for Request Redirect and Retry Methods

[gopalanvinay]

Added unit tests for Redirect methods (withRedirectPolicy, withMaxRedirects) and Retry methods (withRetryPolicy, withMaxRetries)

Resolves: https://github.com/gavv/httpexpect/issues/91

[gavv]

Thanks for the PR!

In these tests, you're checking that fields like req.redirectPolicy, req.retryPolicy, etc. are set correctly. While this is a valid check, it's not really helpful. What is really important is how these fields are used later to construct http.Request struct (not to be confused with httpexpect.Request) - as described in the issue.

When you call Expect(), it will look into redirection parameters and set CheckRedirect and GetBody fields of the http.Request (see setupRedirects()). It will also look into retry parameters and perform retries when request is failed (see retryRequest()).

To test this, we need to develop a mock http client (mockClient may help), pass it instead of a real client (see Config), create httpexpect.Request, configure some redirection/retry parameters, and call Expect(). After that we should inspect how Expect() interacted with our mock client - what were the fields of the generated http.Request, did it perform retries, etc.

If you search for mockClient, you can find examples of other tests using the same approach.

[svaloumas]

Hello @gavv ! Apologies for interfering here. I was experimenting with writing those tests and noticed that in setupRedirects a type assertion on the Client interface takes place at the first line.

func (r *Request) setupRedirects() {
	httpClient, _ := r.config.Client.(*http.Client)

	if httpClient == nil {
		if r.redirectPolicy != defaultRedirectPolicy {
			r.chain.fail("WithRedirectPolicy can be used only if Client is *http.Client")
			return
		}
        ....

Mocking the client out while setting a redirect policy other than the default one, would make the function return before setting the CheckRedirect and GetBody fields, as the httpClient would be nil after the type assertion. I may miss something here, any thoughts?

[gavv]

@svaloumas oh, I see, you're right. We have to use http.Client here. However there is still a place where we can insert mock: we can set http.Client.Transport to our own implementation of http.RoundTripper interface. It is used to actually send request, and there we can check what requests are generated and produce fake responses.

[gavv]

I'm closing this PR, but keeping the issue open (#91). Also I updated the issue text.