Let’s discuss the evolution of testing in the Moby Project – mainly integration-cli tests and their future.
- Most of our integration tests (in
integration-cli package) are using the docker cli to execute — now that this CLI has be exctracted elsewhere we should not depend on it anymore.
- Some tests in this packages are more than just integration tests and could be seen as more end-to-end test of
docker. These should move to our e2e repository and/or have a set of end-to-end tests for moby in the future.
- All the tests are in the same package, in huge files and quick a few of them are a little bit to noisy.
Here is a proposal (that I’ll try to keep up-to-date) of what should happen and in what order — based on our daily in the community slack.
- Freeze
integration-cli package — i.e. not adding anymore tests in there that are using the docker cli. Also add a check to validate that (i.e. if there is any addition in integration-cli, the build fail).
- Create an
integration package for moby api integration tests
- Move
docker_api_*_test.go suites there 
in small packages — i.e. integration/image/build for build-related tests, integration/container/run for container run related tests, integration/distribution for push/pull related tests, etc…
- List tests that in
integration-cli that should move away, either in a end-to-end repository, as api tests or as unit tests.
- For
api tests we should use the client package (it’s unit tested so risks of weirdiness should be limited)
There is more open-question too :
- Should we freeze the
integration-cli or should we extract them in a repo ? Either way, we need a CI job that execute those on moby/moby for an indeterminate time.
Should we use client (docker go sdk) for the API tests or http.Client/integration-cli/request package ? It’s probably a mix of those too but better discussing it.- Which testing framework to use for those integration tests ?
integration-cli uses go-check, should we do the same for integration api tests or should we go back to testing with testify for example ?
If you think there is more point to discuss on that topics, feel free to talk about it, I’ll update that according to the discussions
1 Like
I’m in favor of using http.Client, the docker SDK should have its own testing in its own repo.
Should we freeze the integration-cli or should we extract them in a repo ? Either way, *we need a CI job that execute those on moby/moby for an indeterminate time.
I think we want to “freeze” them in the short term, they are most of our test coverage right now.
Even when docker/e2e repo exists, I assume based on the name, that it will test the docker product, not the moby project, so we would end up with a cyclic dependency between them. Over time components that are removed from moby/moby will have their own tests suites and we should be able to remove some tests.
I see some potential problems/complications with freezing integration-cli
- We will need to make exceptions for fixing test flakes
- We will want to remove tests occasionally, which will probably involved some
+ lines in the diff, even if it’s only removing a test (because of shared code, and table tests)
- Some tests make poor assumptions and could need to be changed because of a backend/api change. Do we require contributors to rewrite the test as an API test at that time?
Which testing framework to use for those integration tests ? integration-cli uses go-check, should we do the same for integration api tests or should we go back to testing with testify for example ?
I’m partial to testify. What does go-check provide?
client/ is not actually an SDK, it’s just a client library. Regardless of the API implementation (http+json REST, gRPC, etc) there is always some client library there. I think the question is more about do we test using the client library or with an HTTP client.
I think the trade off is that the client library is more convenient, but you risk missing flaws in the API because they get hidden by the client library.
If you test with http.Client any API flaws should be more obvious, but you end up duplicating part of what’s in the client library.
Can we get a middle ground where we just have a few helper (i.e. for avoid repeating error checking, and things like encoding/decoding data) instead of a full-fledged client package?
There’s really not much in the client/ package that we would omit. 90% of it is just translating arguments into an HTTP request, and an HTTP response into a struct.
Indeed.
I should have done so before, but the client/ covers a lot less than what I thought. We’d have to check that all parameters can effectively be set via the methods exposed by the client though, since an intermediary type is sometimes used, I assume there may be some gap.
TL;DR: client/ will work for me in the end.
Discussion in the standup; looks like we agree on using client/ for the tests. We will have to update the CONTRIBUTING.md and/or other documents to provide examples on how to write these tests