add docker cleanup to integration tests#2465
Conversation
| ifeq ($(KEEP_COMPOSE),1) | ||
| CLEANUP_COMMAND = echo "Keeping containers running for debugging (KEEP_COMPOSE=1)" | ||
| else | ||
| CLEANUP_COMMAND = docker compose -f dev/docker-compose-integration.yml down -v --remove-orphans 2>/dev/null || true |
There was a problem hiding this comment.
Do you think it could be worth while to also support both docker compose and docker-compose commands?
There was a problem hiding this comment.
Hey @gabeiglio, I'd say let's stay with compose v2 (docker compose) support, since the v1 (docker-compose) is EOL, and docker keeps an alias so old scripts continue to work already.
WDYT?
There was a problem hiding this comment.
I would not be in favor of supporting the EOL docker-compose
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
I'm okay with this change since it is cleaner to close the containers. Thanks for making it configurable, since I like to be able to run tests in my IDEA for debugging (if something fails).
kevinjqliu
left a comment
kevinjqliu
left a comment
There was a problem hiding this comment.
Ran make test-integration locally, confirmed docker containers are gone.
Thanks!
4 participants
related to #2425
Rationale for this change
This PR updates the Makefile's test-integration target to cleanup the docker containers by default. Also, adds a variable (
KEEP_COMPOSE) that can ignore the cleanup for development purposes. but the setupAre these changes tested?
yes
Are there any user-facing changes?
no