docker_network: add basic integration tests #46137
Conversation
ansibot
added
affects_2.8
|
CC @kassiansun @pilou- since you might have some general interest in this, too |
| msg: "Using name prefix {{ name_prefix }}" | ||
|
|
||
| - block: | ||
| - include_tasks: run-test.yml |
There was a problem hiding this comment.
use - include_tasks: "{{ item }}" directly, no need to use run-test.yml (- include_tasks: tests/basic.yml works too)
There was a problem hiding this comment.
There's one big advantage over doing it this way: if you do all include_tasks directly, you will get the included: .../tasks/tests/xxx.yml for localhost all at the same time, while with this indirect include, you will get them before the tasks are actually executed. This makes it easier to find out in which file a task is defined (if you're hunting down something that breaks).
There was a problem hiding this comment.
(I originally did it directly for the docker_container tests, but after I had two tests instead of only one, I noticed this and changed it this way to make it easier to find out to which "sub-test" tests belong to.)
There was a problem hiding this comment.
Nothing to change here, I would have done differently (maybe using debug module :)
| name: "{{ item }}" | ||
| state: absent | ||
| stop_timeout: 1 | ||
| with_items: "{{ cnames }}" |
There was a problem hiding this comment.
You might want to use loop here (it is already used in basic.yml).
There was a problem hiding this comment.
Thanks! Fixed.
| nname_2: "{{ name_prefix ~ '-network-2' }}" | ||
| - name: Registering container name | ||
| set_fact: | ||
| cnames: "{{ cnames }} + [cname_1, cname_2, cname_3]" |
There was a problem hiding this comment.
Isn't cnames: "[cname_1, cname_2, cname_3]" sufficient ? Then set_fact: cnames at beginning of main.yml could be removed.
There was a problem hiding this comment.
Now there's only one sub-test (basic.yml), so yes, that would work, but as soon as one more is added, this is necessary (so that all names are collected, and the objects will be destroyed even in case of failing tests and other errors).
There was a problem hiding this comment.
Not sure what this future test will look like :)
| name: "{{ container_name }}" | ||
| image: hello-world | ||
| state: present | ||
| loop: |
There was a problem hiding this comment.
loop: {{ cnames }} could be used here
There was a problem hiding this comment.
Only at this point where there's only one sub-test.
| - "{{ cname_3 }}" | ||
| loop_control: | ||
| loop_var: container_name | ||
|
|
There was a problem hiding this comment.
no idempotency check for container ?
There was a problem hiding this comment.
No, that's already done enough many times in the docker_container integration tests ;)
| @@ -0,0 +1,117 @@ | |||
| --- | |||
| - name: Registering container name | |||
There was a problem hiding this comment.
Thanks! Fixed.
jborean93
removed
the
needs_triage
ansibot
added
the
stale_ci
|
bot_status |
Componentstest/integration/targets/docker_network/aliases test/integration/targets/docker_network/meta/main.yml test/integration/targets/docker_network/tasks/main.yml test/integration/targets/docker_network/tasks/run-test.yml test/integration/targets/docker_network/tasks/tests/basic.yml Metadatawaiting_on: maintainer |
felixfontein
force-pushed
the
docker_network-tests
branch
from
40a9efb
to
aa6eb56
Compare
ansibot
removed
the
stale_ci
|
bot_status |
Componentstest/integration/targets/docker_network/aliases test/integration/targets/docker_network/meta/main.yml test/integration/targets/docker_network/tasks/main.yml test/integration/targets/docker_network/tasks/run-test.yml test/integration/targets/docker_network/tasks/tests/basic.yml Metadatawaiting_on: maintainer |
|
LGTM |
ansibot
added
the
stale_ci
|
Getting rid of stale_ci. |
|
bot_status |
Componentstest/integration/targets/docker_network/aliases test/integration/targets/docker_network/meta/main.yml test/integration/targets/docker_network/tasks/main.yml test/integration/targets/docker_network/tasks/run-test.yml test/integration/targets/docker_network/tasks/tests/basic.yml Metadatawaiting_on: maintainer |
ansibot
removed
the
stale_ci
|
@dariko @jwitko @kassiansun @tbouvet Can anyone of you take a look here? |
|
shipit |
|
bot_status |
Componentstest/integration/targets/docker_network/aliases test/integration/targets/docker_network/meta/main.yml test/integration/targets/docker_network/tasks/main.yml test/integration/targets/docker_network/tasks/run-test.yml test/integration/targets/docker_network/tasks/tests/basic.yml Metadatawaiting_on: maintainer |
|
Hmmm, shouldn't kassiansun count as a namespace maintainer? |
|
Something similar to this PR is included in #35370. |
ansibot
added
the
community_review
|
bot_status |
Componentstest/integration/targets/docker_network/aliases test/integration/targets/docker_network/meta/main.yml test/integration/targets/docker_network/tasks/main.yml test/integration/targets/docker_network/tasks/run-test.yml test/integration/targets/docker_network/tasks/tests/basic.yml Metadatawaiting_on: maintainer |
|
Yay for tests, thank you. Maybe worth backporting this to |
|
Thanks everyone for reviewing and getting this merged! |
* Adding very basic integration tests for docker_network. * Fixing some details (review comments). (cherry picked from commit 131efcf)
* Adding very basic integration tests for docker_network. * Fixing some details (review comments). (cherry picked from commit 131efcf)
* Adding very basic integration tests for docker_network. * Fixing some details (review comments).
SUMMARY
So far, there are no tests for docker_network at all. Having some would definitely be nice for #43997.
ISSUE TYPE
COMPONENT NAME
docker_network
ANSIBLE VERSION