RFC: Testing docker.Transport using recorded requests/responses

[mtrmac]

Add infrastructure and an example for testing dockerImageSource via github.com/dnaeon/go-vcr/recorder .

This allows running the test against a live registry once, manually inspecting the requests/responses to be as expected, and then running all tests against a “recording” of the requests/responses, completely
off-line.

Also adds a simple test for GetManifest to demonstrate the use of the mechanism.

The httpWrapper mechanism is intended purely for tests, and not intended to be
exposed to external users; they should use types.SystemContext

[mtrmac]

@runcom PTAL before I go with this approach to testing containers/image/docker any further. This is a proposed alternative to #44 and that old discussion, notably

• it is at a much lower level, raw HTTP vs. dockerClient
• Instead of writing mock HTTP responses (manual work with fake data), the tests would just record real-world interactions (less manual work, only to set up the servers, and real data).
• The downside to this is that the tests would also break every time we change anything about the request, e.g. add a header, and we would need to update the recordings in such a case. OTOH that means that we will actually test the behavior, and that the complete behavior change (both request and response) will be reviewed as part of the PR.

There are various “vcr” libraries for Go. My critical requirement was the ability to run the tests off-line, from recordings only; most other libraries fall back to silently communicating over the internet if the request is not in the existing recording, which makes it too easy to not notice that we have changed behavior.

We can, and do, test against live real-world servers, in the skopeo integration tests. This is intended to be purely local, off-line, small-scale and very quick to execute.

[mtrmac]

See #255 for a reasonably heavy use of this mechanism.

That has run into dnaeon/go-vcr#21, which should hopefully not be too difficult to fix.

Admittedly updating the tests for OpenShift is somewhat of a pain due to how setting up OpenShift is not quite automated (containers/skopeo#320 helps, but we could do more) and how the various signatures and signature names are random/time-dependent (similarly, we could use more fixtures with fixed contents). So, re-recording the tests requires updating various items (like passwords and manifest digests) which may be quite unrelated to the reason why the tests are being re-recorded.

During development of the code, this seems well worth it—it would be necessary to set up a testing environment anyway, and this way the one-time setup is quite effortlessly turned into an automated regression test.

I expect the major drawbacks to this method to manifest when changing dockerClient (e.g. adding a header), requiring all tests to be re-recorded, even if the person making the change has no interest in a particular registry server implementation. I guess forcing such people to care anyway might be sold as a benefit as well ;-)

[runcom]

I still believe, in Golang, it's better to have an interface for this where it could be mocked in testing. This could be far better instead but far from what I think golang tests for this should be.

[mtrmac]

I still believe, in Golang, it's better to have an interface for this where it could be mocked in testing.

An interface for which “this”? dockerClient or http.*?

(The httpWrapper does allow substituting a completely fake http.RoundTripper, it does not require the use of go-vcr.)

Mocking dockerClient is a bit awkward in that the authentication behavior is one of the essential components to be tested. Though i can see the argument that there should be a separate test set for the authentication, which uses a real dockerClient, and a separate test set which assumes sufficient authentication and just tests the individual read/write operations, which uses a mock dockerClient for simplicity. (I am not sure that it can be safely factored that way, but I can see how it is attractive.)

[Also, #255 adds helper methods shared between the source/destination to dockerClient, which would make the mocking of dockerClient awkward; but they could just as well be methods which take a dockerClient parameter.]

This could be far better instead but far from what I think golang tests for this should be.

I guess let's have a conversation about what the tests should be? on IRC, or perhaps over the phone?

I am honestly unsure about this approach. I do like that it is pretty close to real-world request/responses, and that it is easy to write the tests (just write client calls, the real server will provide real responses); OTOH there is substantial risk that maintaining the recordings will be a pain.

[runcom]

apart from my personal opinion, I'm ok doing this.

[mtrmac]

apart from my personal opinion, I'm ok doing this.

That‘s “apart from no, yes”? :) Let's find a good solution, I don’t want to force this approach through.

[rhatdan]

@mtrmac last update on this was over a year ago, do we still care about it?

[mtrmac]

*shrug* It still seems a potentially useful. Rebased, at least.

[mtrmac]

Not valuable enough.