docker: move dockerClient to an interface

[runcom]

This should be a less opinionated change based on #30.

Unit testing docker_image_src.go and docker_image_dest_go should be easier after this as we can mock up the entire client (along the line of d5cbfe9).

This change goes along with #42 where we set the actual client in the dockerClient struct - thus, allowing more fine grained unit tests like the ones expressed in d5cbfe9#r69351589. Arguably I think the same unit tests can be done with just #42 because we can mock the entire client there as well.

@mtrmac PTAL, eventually using Go httptest package is what we'll end up - but this ease the burden of having to setup an httptest server everytime with different settings and handlers and just mocking the whole makeRequest* game. We can still use httptest in a docker_client_test.go to test authentication, ping, https/http detection and what not.

Signed-off-by: Antonio Murdaca runcom@redhat.com

[mtrmac]

Sure, this is fine. Do you want to fold this this in a PR which adds an actual user of the abstraction?

WFM either way, needs a rebase though.

[runcom]

re-iterating on this - this seems still not the right approach as even this is requiring tests to implement N client(s) where N is every possible action/output of makeRequest* based on what we're testing. I'm looking for another approach to tackle this though.. any idea?

[mtrmac]

re-iterating on this - this seems still not the right approach as even this is requiring tests to implement N client(s) where N is every possible action/output of makeRequest* based on what we're testing.

Is that a barrier? Something vaguely like this would work, wouldn’t it?

type dockerClientMock func(method, url string, headers map[string][]string, stream io.Reader) (*http.Response, error)

func (fn dockerClientMock) makeRequestToResolvedURL(…) (…) {
   return fn($the_params)
}

func (fn dockerClientMock) makeRequest(…) (…) {
   url = pretendWeAreResolving(url)
   return fn($the_params)
}

and then each test would call something like

src := newImageSource(ref, dockerClientMock(func(…) (…) {
   returnSimulatedResponse
}))
assert.Equal(t, expectedResult, src.SomeOperation())

Or perhaps the helper type could do even more, perhaps there could be a

type expectingSingleHTTPMock { method, url string, []byte result }

if that did cover most of the cases (which it probably doesn’t).

Another possibility would be to split things like

func (…) GetBlob(…) (…) {
   req = prepareGetBlobRequest($params)
   res := client.doHTTP(req)
   return parseGetBlobResponse(res)
}

and at this level unit test only prepareGetBlobRequest and parseGetBlobResponse.

But overall, I don’t think I can guess which one is most practical / results in most readable code in actual tests. My best idea of figuring this out would be to start writing tests and then keep refactoring until they look nice (… but then policy_config_test.go is clearly not looking nice; my excuse is that I didn’t want to spend time making that perfect in this sense :-) )

[rhatdan]

Since this has not been updated in almost 2 years, can we close it?