Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e2e: ensure all Compose cmds standalone compatible #9567

Merged
merged 2 commits into from
Jun 16, 2022
Merged

Conversation

milas
Copy link
Contributor

@milas milas commented Jun 16, 2022

What I did
The E2E tests can be run in plugin (docker compose) or standalone
(docker-compose) mode. Existing logic was in place to ensure that
the helper method is always used, which will invoke the right one
based on how tests are being executed.

However, this logic was too easy to (unintentionally) bypass given
the myriad of ways that commands can be run. The check has been
made stricter and pushed to a lower-level to more aggressively
catch instances.

As a result, a bunch of calls to RunDockerCmd are now updated
to be RunDockerComposeCmd, which will ensure that the invocation
is correct based on test mode.

(not mandatory) A picture of a cute animal, if possible in relation with what you did
field mouse

The E2E tests can be run in plugin (`docker compose`) or standalone
(`docker-compose`) mode. Existing logic was in place to ensure that
the helper method is always used, which will invoke the right one
based on how tests are being executed.

However, this logic was too easy to (unintentionally) bypass given
the myriad of ways that commands can be run. The check has been
made stricter and pushed to a lower-level to more aggressively
catch instances.

As a result, a bunch of calls to `RunDockerCmd` are now updated
to be `RunDockerComposeCmd`, which will ensure that the invocation
is correct based on test mode.

Signed-off-by: Milas Bowman <[email protected]>
Copy link
Member

@nicksieger nicksieger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

This was using `docker exec` on Compose containers instead of
`docker compose exec` (and `docker-compose exec` for standalone).

Thanks to @glours for catching!

Signed-off-by: Milas Bowman <[email protected]>
Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@glours glours enabled auto-merge June 16, 2022 21:00
@glours glours merged commit 4270987 into docker:v2 Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants