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 all fixtures to be function scoped #91

Open
skshetry opened this issue Nov 9, 2022 · 2 comments
Open

make all fixtures to be function scoped #91

skshetry opened this issue Nov 9, 2022 · 2 comments

Comments

@skshetry
Copy link
Member

skshetry commented Nov 9, 2022

We should provide function scoped by default for correctness, and we can offer fixtures with a different scope if needed.

Ideally, it'd look something like this:

def s3_server_impl():
   yield MockedS3Server()

@pytest.fixture():
def s3_server():
   yield s3_server_impl()

@pytest.fixture(scope="session"):
def session_s3_server():
   yield s3_server_impl()

...
@shcheklein
Copy link
Member

I hit an issue in trying to use pytest-servers in Studio tests. My 2cs (from a very brief interaction and research) - it's fine that server is running once per session to my mind (it takes at least a few seconds to launch it). But what hit me is that we are modifying environment variables (to pass secrets, etc) - that can conflict with some existing tests that are using regular S3 or minio or something else.

Can we start by isolating that first? Can we wrap credentials as a separate function scoped fixture that clients could use if it's needed.

WDYT, folks?

@skshetry could give more motivation behind making the server function scoped?

Also, pytest supports dynamic scopes - we might take a look into it.

@dtrifiro
Copy link
Contributor

dtrifiro commented Dec 15, 2023

Hey @shcheklein

In #137 I got rid of the session-scoped s3_fake_creds_file while keeping the s3 server session-scoped. This should make configuration of the fixture easier, as well avoiding patching env vars.

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

No branches or pull requests

3 participants