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

Rethink how env vars are mocked in tests #316

Open
rmanaem opened this issue May 28, 2024 · 3 comments
Open

Rethink how env vars are mocked in tests #316

rmanaem opened this issue May 28, 2024 · 3 comments

Comments

@rmanaem
Copy link
Collaborator

rmanaem commented May 28, 2024

  • How do we test an os.env read?
  • How do we test that changing the env var actually results in the functions in the test having the new value?

For testing the time at which the env vars are read is at import time as opposed to at app start time/test runtime.

In the n-API (and soon f-API: neurobagel/federation-api#90) we load ENV variables in a utility module like so

api/app/api/utility.py

Lines 16 to 18 in 84c0e99

ALLOWED_ORIGINS = EnvVar(
"NB_API_ALLOWED_ORIGINS", os.environ.get("NB_API_ALLOWED_ORIGINS", "")
)

Because the loading happens at the top level of the module, the ENV variable gets read in when the module is imported. In other words: we have no control over when the ENV variable is read in (e.g. during or after app startup).

This mainly matters during testing where we have to make sure we set ENV variables before the test run starts at all using something like https://pypi.org/project/pytest-env/ because by the time we have launched the TestApp client or imported the utility module any further changes to ENV variables will have no effect (because the reading has already happened).

This has two immediate consequences:

  • it's tricky / hard to actually test the reading of ENV variables (e.g. testing for several ENV values)
  • we need hard-to-fully-understand workarounds like monkeypatching the internal variable that the ENV value is read into

Possible solutions

@surchs surchs transferred this issue from neurobagel/federation-api May 28, 2024
@surchs
Copy link
Contributor

surchs commented May 28, 2024

Transferred this to n-API because we use env vars here more so far.

Copy link

We want to keep our issues up to date and active. This issue hasn't seen any activity in the last 75 days.
We have applied the _flag:stale label to indicate that this issue should be reviewed again.
When you review, please reread the spec and then apply one of these three options:

  • prioritize: apply the flag:schedule label to suggest moving this issue into the backlog now
  • close: if the issue is no longer relevant, explain why (give others a chance to reply) and then close.
  • archive: sometimes an issue has important information or ideas but we won't work on it soon. In this case
    apply the someday label to show that this won't be prioritized. The stalebot will ignore issues with this
    label in the future. Use sparingly!

@github-actions github-actions bot added the _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again label Aug 12, 2024
@alyssadai
Copy link
Contributor

See also pytest-dev/pytest#10027 (comment)

@alyssadai alyssadai added the flag:schedule Flag issue that should go on the roadmap or backlog. label Oct 25, 2024
@github-actions github-actions bot removed the _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again label Oct 26, 2024
@surchs surchs removed the flag:schedule Flag issue that should go on the roadmap or backlog. label Oct 28, 2024
@alyssadai alyssadai changed the title Env var in tests Rethink how env vars are mocked in tests Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

3 participants