Skip to content

Add --abort-on-container-failure option #1190

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

Merged
merged 1 commit into from
May 10, 2025

Conversation

gtebbutt
Copy link
Contributor

@gtebbutt gtebbutt commented Apr 30, 2025

Fixes #1164, with behaviour matching docker/compose#11680 (see discussion in docker/compose#10225)


class TestComposeAbort(unittest.TestCase, RunSubprocessMixin):
def test_abort_on_exit_fail_first(self):
self.run_subprocess_assert_returncode(
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to shutdown the containers with compose down after the test.

Also, consider using parameterized. Pass the scenario name as the first argument name and don't use it.

see e.g. tests/integration/lifetime/test_lifetime.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, thanks for the notes. I skipped the name parameter because the parameters seemed fairly self explanatory once they were written out, but happy to add that explicitly if you think it's helpful.

Copy link
Collaborator

@p12tic p12tic left a comment

Choose a reason for hiding this comment

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

Overall looks good, had a comment about tests. Thanks a lot!

@p12tic p12tic force-pushed the abort-on-failure branch from 921b7ea to e1d938f Compare May 10, 2025 10:41
@p12tic
Copy link
Collaborator

p12tic commented May 10, 2025

I combined commits into one because later commits fix the first commit.

Copy link
Collaborator

@p12tic p12tic left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@p12tic p12tic merged commit 6acdafd into containers:main May 10, 2025
8 checks passed
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.

Support for --abort-on-container-failure (to match docker compose)
2 participants