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

Make container scopes configurable fix solution #103

Merged

Conversation

BetterCallBene
Copy link
Contributor

Resolves #89

n1ngu

This comment was marked as resolved.

@n1ngu
Copy link
Contributor

n1ngu commented Jan 24, 2024

Its sort of stale, but what's the difference with #91 ?

@BetterCallBene
Copy link
Contributor Author

Its sort of stale, but what's the difference with #91 ?

I agree it is not too much but as the comment #91 (comment) said the mr is not working. I found out the pytest_addoptions is on the wrong place and the mr request was containing a typo and haven‘t any integration tests

@BetterCallBene
Copy link
Contributor Author

Also, the documentation about this in the Readme.md should be changed. Now it reads

All fixtures have session scope.

A change such as this should be properly documented there.

See https://github.com/avast/pytest-docker/#available-fixtures

I will update the README.md, too.

setup.cfg Outdated
@@ -1,6 +1,6 @@
[metadata]
name = pytest-docker
version = 2.0.1
version = 2.1.0

This comment was marked as resolved.

return request.config.getoption("--container-scope")

def containers_scope(fixture_name, config):
return config.getoption("--container-scope", "module")

This comment was marked as resolved.

This comment was marked as resolved.

@BetterCallBene
Copy link
Contributor Author

Hey @n1ngu, please have look again. Thanks

Copy link
Contributor

@n1ngu n1ngu left a comment

Choose a reason for hiding this comment

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

LGTM! Note I have no powers on this repo and can't merge nor even trigger the workflows. Let's see if we can draw the attention of a real maintainer.

@Luminaar @augi sorry to bother you: it seems that the pytest-docker community is interested in this feature. @austinkeller @gipi @BetterCallBene and me tried our best to get this done, if you kindly give it a look IMHO the present PR is ready to go.

My only remaining concern would be the 8 commits drift so I'd advise a squash+merge.

Thanks!

@augi augi merged commit a001fd0 into avast:master Jan 25, 2024
20 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.

Allow for overriding of pytest fixture scope
3 participants