Adding a Makefile and basic tests.#86
Adding a Makefile and basic tests.#86metcalfc wants to merge 8 commits intodocker:masterfrom metcalfc:bats
Conversation
This approach uses bats to create a generic docker-compose test. It checks that images are pullable, projects are buildable, up works, and if ports are exposed that something is listening. The tests can be tailored for each example. As long as they are the same though, you can edit lib/test.bats.example and then `make update-tests` and all the tests will be synced. Signed-off-by: Chad Metcalf <chad@docker.com>
Signed-off-by: Chad Metcalf <chad@docker.com>
Signed-off-by: Chad Metcalf <chad@docker.com>
[ "$status" -eq 0 ] just bails out and its up to you to figure out why. $output has the stdout/stderr so we could just print that. I think its worth vendoring the libraries and just using those functions. Signed-off-by: Chad Metcalf <chad@docker.com>
Signed-off-by: Chad Metcalf <chad@docker.com>
The full test suite takes a long time to test. Generally changes in this repo only will change one project. So lets find the directories with changes and only test those. By default we will compare against origin/master which should cover testing PRs. If you're doing development and want to test your current changes you could do: BASE_REF=HEAD make test-changed Or if you're testing against your fork: BASE_REF=myremote/branch make test-changed Signed-off-by: Chad Metcalf <chad@docker.com>
Signed-off-by: Chad Metcalf <chad@docker.com>
glours
left a comment
There was a problem hiding this comment.
Hi @metcalfc
Can't we use the Docker image of bats to do that instead of adding the whole bats source code in the lib directory?
Because all the test files are identical, it should be interesting to have only 1 test file and to loop over directories to execute tests.
In that way we'll only have 1 test file to maintain and we'll sure that a new sample will be automatically covered by test
|
The Dockerfile approach would require a docker in docker setup. I didnt tackle that out of the gate because its a PITA. If this approach works for folks we could add it later.
The vendored code in lib are libraries for bats. You still have to install bats locally or run the docker image.
I thought about the single test but thought eventually we would tailor the tests to each project. The current generic test is useful to start. Eventually though we could add tests for expected output, http status codes (vs generic nc), test api endpoints, etc. At this point they’d diverge. The generic could be enough really depends on what level of fidelity we want. This wont catch changes that break the app but leave the app in a running state which seems like a miss.
Also bats is pretty opinionated about where the tests are defined and run from. For example its not possible to generate tests on the fly (or at least i couldnt make it work). Initially I tried to generate individual port tests but the way bats works with multiple passes for each test that doesnt work because the tests get renamed as they are generated and bats files are “run” once for every test. Bats is sort of magic and its not as simple as its just a script.
Folks could decide that bats isnt the route and instead we should for example take a node/npm dependency and write a library to shell out, etc that just seemed like a lot of work vs bash.
… On Aug 10, 2020, at 6:40 AM, Guillaume Lours ***@***.***> wrote:
@glours commented on this pull request.
Hi @metcalfc
Can't we use the Docker image of bats to do that instead of adding the whole bats source code in the lib directory?
Because all the test files are identical, it should be interesting to have only 1 test file and to loop over directories to execute tests.
In that way we'll only have 1 test file to maintain and we'll sure that a new sample will be automatically covered by test
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
Over a 🥃 I spent some time trying to put all this in docker. I wanted to see if I could get away without full dind. The dockerfile: A couple of Gets you part way there. Unfortunately when it comes to testing the network ports the external network isn't available. So all those checks will fail. Unless I've missed something you would need a full dind setup. I can play around with it more when I get back from vacation. |
|
@glours Walking the 🐕 with a ☕️ and |
I've seen a lot more failures. Compose is not the latest. It seems to sort of work. We should probably parameterize Docker and Compose versions to ensure we're installing a consistent toolchain. Signed-off-by: Chad Metcalf <chad@docker.com>
|
Awesome thanks @metcalfc 👍 |
This approach uses bats to create a generic docker-compose test. It
checks that images are pullable, projects are buildable, up works, and
if ports are exposed that something is listening.
The tests can be tailored for each example. As long as they are the same
though, you can edit lib/test.bats.example and then
make update-testsand all the tests will be synced.
This adds three project dependencies: Makefile, BATS, and nc. A full clean
test run is PAINFULLY long (~14 minutes on my desktop) mostly
dominated by the build steps.
I've also added a target to just test whats changed. Since most of the time
PRs will just change one project. By default git finds the directories with
changes compared to
origin master. You can override this withBASE_REF=<whatever> make test-changed.