Feature: Handle binary response body

[rafaeljusto]

When retrieving a binary response body, convert it to hexadecimal so we can safely add it to the snapshot file.

[sjkaliski]

I'm wondering if we should rethink the method signature so it can accommodate all response types. Otherwise it'll start to become overloaded as we support other response types. Maybe the method determines the content-type on its own?

[rafaeljusto]

Yeah, totally agree with you. It will be a pain to keep track of all content-types.

Maybe the method determines the content-type on its own?

You mean, the method will receive an enum? Like:

// JSON
abide.AssertHTTPResponse(t, t.Name(), w.Result(), abide.ResponseTypeJSON)

// text
abide.AssertHTTPResponse(t, t.Name(), w.Result(), abide.ResponseTypeText)

// binary
abide.AssertHTTPResponse(t, t.Name(), w.Result(), abide.ResponseTypeBinary)

We could use functional options to keep backward compatibility.

// ResponseType expected response body type.
type ResponseType string

// ResponseType possible values.
const (
  ResponseTypeJSON   ResponseType = "json"
  ResponseTypeText   ResponseType = "text"
  ResponseTypeBinary ResponseType = "binary"
)

// SetResponseType define the response type in the options.
func SetResponseType(typ ResponseType) func(*AssertOptions) {
  return func(options *AssertOptions) {
    options.ResponseType = typ
  }
}

// AssertOptions possible options for the assert.
type AssertOptions struct {
  ResponseType ResponseType
}

// AssertHTTPResponse asserts the value of an http.Response.
func AssertHTTPResponse(t *testing.T, id string, w *http.Response, opts ...func(*AssertOptions)) {
  options := &AssertOptions{}
  for _, opt := range opts {
    opt(options)
  }
  // ...
}

abide.AssertHTTPResponse(t, t.Name(), w.Result(), abide.SetResponseType(abide.ResponseTypeBinary))

We could have better names for sure 😄 (I'm terrible at giving names to things). But we would still need to check for the JSON content-type to keep backward compatibility when the ResponseType is empty. What do you think?

[sjkaliski]

@rafaeljusto this sounds great and let's go down this route!

We should also figure out the module/versioning plan here long term. I'll file a ticket.

[rafaeljusto]

@sjkaliski Applied the changes there. Do you think we should just hex encode binary content or use some hash function? Because the content body can be big and not really comparable 😄

[sjkaliski]

Thanks! Will review tonight/tomorrow!