refactored caddytest helpers

[sarge]

Refactored the caddytests helpers. I am shooting for making the simple cases easy, and the complex ones possible.

I have exposed a low level function

func AssertResponseCode(t *testing.T, req *http.Request, expectedStatusCode int) *http.Response {

And have build a some of the higher level functions for some of the common cases. Building wrappers for all of the http verbs seems unnecessary.

@greenpau would you mind having a look and seeing if these are suitable?

[greenpau]

@sarge , thank you for exposing CreateTestingTransport. I will contribute the AssertPostResponseBody in a bit.

Generally, I think client needs to be exposed to give full control of it to a tester. For example setting user agent ahead of time and keeping the client state as one progresses through tests.

I propose it to look something like this.

type TestClient struct {
  client *http.Client // not being exposed
  t *testing.T 
}

func NewTestClient(t *testing.T) *TestClient {
  return &TestClient{
    client: &http.Client{
      CheckRedirect: redirectPolicyFunc,
      Transport:     CreateTestingTransport(),
  }
}

Then, if a tester want to use AssertGetResponse as it currently stands, they can.

func AssertGetResponse(t *testing.T, requestURI string, statusCode int, expectedBody string) (*http.Response, string) {

The TestClient gets its own method that

func (*TestClient) AssertGetResponse( ...
.... the client already has `t *testing.T`.

This would be backwards compatible because the logic for client is actually in AssertGetResponseBody. You extend AssertGetResponseBody to include client *TestClient:

func AssertGetResponseBody(client *TestClient, requestURI string, expectedStatusCode int) (*http.Response, string) {

After that, AssertGetResponse would look like this:

func AssertGetResponse(t *testing.T, requestURI string, statusCode int, expectedBody string) (*http.Response, string) {
        client := NewTestClient(t *testing.T)
	resp, body := AssertGetResponseBody(client, requestURI, statusCode)
	if !strings.Contains(body, expectedBody) {
		t.Errorf("requesting \"%s\" expected response body \"%s\" but got \"%s\"", requestURI, expectedBody, body)
	}
	return resp, body
}

Please let me know your thoughts.

[sarge]

@greenpau thanks for the feedback and suggestions. Interesting your observations about control over the http client. I had been considering an approach similar to the http.DefaultClient adding that as something you can do but I suspect seldom need to. It is global state, but it is familiar.

Something I just needed to look up was about 'keeping the client state as one progresses through tests'. As I understand it cookies won't be remembered, what other state were you hoping to track? In general useragent should not be encouraged to drive behaviour, though I can see in practice it might be. caddytest.DefaultClient would give you that.

I also wonder how deep into http testing caddy really needs to get. It seems we will need to make a call about all the other httpverbs? I definitely wanted to shoulder the complexity of starting and loading configuration.

Not many answers here I know

[greenpau]

@sarge , i implemented it here. Please Review #3299

[greenpau]

cookies won't be remembered

@sarge , I think with this approach you can extend it to have a cookie jar (https://golang.org/pkg/net/http/cookiejar/)

[sarge]

@greenpau I have taken another pass through these changes. I had added a cookie jar and a functioning test. I have also added few more verbs.

Take a look I think I have captured the spirit of what you needed.

[greenpau]

Take a look I think I have captured the spirit of what you needed.

@sarge , LGTM 👍

[greenpau]

@sarge , LGTM 👍

[greenpau]

@sarge , I approved the changes, but it looks like it does not count myself as an approver.

cc: @mholt