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

Fix tests for build environment relying on PYTHONPATH #318

Merged
merged 1 commit into from
Apr 29, 2021

Conversation

befeleme
Copy link
Contributor

Hey,
I want to update dotenv to 0.17.0 in Fedora, but realized the two newly introduced tests started failing in the build process.
This happens because defining _env replaces the existing environment, effectively unsetting many variables (which, in my case, means losing information where to look for the module to import). Instead of creating completely new contents, I propose copying existing environment and adding the tested key-value on the top of it.
This will not affect the existing test suite but can help other packagers.

Copy link

@hroncok hroncok left a comment

Choose a reason for hiding this comment

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

I think some of the environment variables got accidentally lost.

tests/test_cli.py Outdated Show resolved Hide resolved
tests/test_cli.py Outdated Show resolved Hide resolved
@befeleme befeleme force-pushed the copy-existing-env branch 2 times, most recently from d9fae2a to 13303ca Compare April 29, 2021 09:24
@befeleme
Copy link
Contributor Author

Thanks for the suggestion, fixed.

Copy link

@hartwork hartwork left a comment

Choose a reason for hiding this comment

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

Very nice, I was about to push the same thing for Gentoo 👍

The missing variable is PYTHONPATH: Without it setuptools easy-install-entry-script will happily use (say) dotenv 0.16.0 installed in the system rather than 0.17.0 in those two tests. So my vote for merging this before the next release. Thank you! 🙏

Related:

@hartwork
Copy link

PS: @befeleme could you adjust the title of the pull request to make clear that this is a bugfix for the tests? Thank you!

Overriding the whole environment would remove critical variables like
`PYTHONPATH`. This would break tests on some systems like during Fedora
or Gentoo packaging.
@bbc2 bbc2 force-pushed the copy-existing-env branch from 13303ca to b52b3f5 Compare April 29, 2021 18:56
@bbc2 bbc2 changed the title Copy existing environment for usage in tests Fix tests for build environment relying on PYTHONPATH Apr 29, 2021
@bbc2
Copy link
Collaborator

bbc2 commented Apr 29, 2021

Thank you for the fix! I just fixed some formatting and updated the changelog. I'll release this in a few minutes.

@bbc2 bbc2 merged commit 7d9b45a into theskumar:master Apr 29, 2021
@bbc2
Copy link
Collaborator

bbc2 commented Apr 29, 2021

Released as v0.17.1.

@hartwork
Copy link

Very nice, thank you! 👍 👍

@befeleme befeleme deleted the copy-existing-env branch April 30, 2021 05:45
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.

4 participants