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: enables default encoding warning to ensure users set encoding #3619

Merged

Conversation

sharyar
Copy link
Contributor

@sharyar sharyar commented Apr 22, 2023

Adds the PYTHONWARNDEFAULTENCODING=1 to tox.ini as an env variable to enable warnings for encodings. See also https://peps.python.org/pep-0597/

@Zac-HD Zac-HD added interop how to play nicely with other packages internals Stuff that only Hypothesis devs should ever see labels Apr 22, 2023
@sharyar
Copy link
Contributor Author

sharyar commented Apr 23, 2023

@Zac-HD I think I might need some help with some of the failing tests. I think they are coming from external libraries but I am not 100% certain.

@Zac-HD
Copy link
Member

Zac-HD commented Apr 23, 2023

Looks like we need to convert over the fixtures in hypothesis-python/tests/ghostwriter/test_ghostwriter_cli.py too, then I think we're probably done!

I'd also make it a patch release rather than minor, since from a user perspective this is just a bugfix. (and the test changes don't need to be mentioned at all)

@sharyar
Copy link
Contributor Author

sharyar commented Apr 23, 2023

@Zac-HD any chance you could checkout the failing tests? I suspect this has something to do with black now.

@Zac-HD Zac-HD force-pushed the python_default_encoding_warning branch from 0b5d28e to a218f05 Compare April 24, 2023 06:34
Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

Thanks again Sharyar!

I cherry-picked the AUTHORS entry and in-package change into the latest release, have rebased this, and then disabled the new warning for Pytest 6.2 (before it was handled there) and the Ghostwriter tests (I think Black hasn't enabled the warning yet, psf/black#3658). So once CI passes, this should be ready to merge!

@Zac-HD Zac-HD enabled auto-merge April 24, 2023 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internals Stuff that only Hypothesis devs should ever see interop how to play nicely with other packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants