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 up some pytest style issues #6968

Merged
merged 7 commits into from
Mar 3, 2023
Merged

Conversation

akx
Copy link
Contributor

@akx akx commented Feb 23, 2023

This PR fixes tests to use pytest.raises and pytest.warns as context managers everywhere possible.

@akx akx marked this pull request as draft February 23, 2023 13:53
@akx akx marked this pull request as ready for review February 23, 2023 14:20
@hugovk hugovk changed the title Fix up some py.test style issues Fix up some pytest style issues Feb 23, 2023
@hugovk hugovk added the Testing label Feb 23, 2023
Tests/test_file_tiff.py Outdated Show resolved Hide resolved
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@radarhere
Copy link
Member

Is there a link that you can provide, describing this pytest style preference?

@akx
Copy link
Contributor Author

akx commented Feb 25, 2023

@radarhere Kind of:

(Pillow) ~/b/Pillow (main) $ git grep pytest.raises | grep with | wc -l
     505
(Pillow) ~/b/Pillow (main) $ git grep pytest.raises | grep -v with | wc -l
      18
(Pillow) ~/b/Pillow (main) $ git grep pytest.warns | grep with | wc -l
      48
(Pillow) ~/b/Pillow (main) $ git grep pytest.warns | grep -v with | wc -l
      27

IOW, Pillow's tests themselves overwhelmingly use with pytest.raises/with pytest.warns.

@hugovk
Copy link
Member

hugovk commented Feb 25, 2023

And I find im.save(out, save_all=True, append_images=ims) cleaner/more readable and closer to normal use than a big list of parameters: UserWarning, im.save, out, save_all=True, append_images=ims.

@akx
Copy link
Contributor Author

akx commented Mar 2, 2023

Could this be merged? CI is green across the board.

radarhere and others added 2 commits March 3, 2023 10:28
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
Tests/test_file_webp.py Outdated Show resolved Hide resolved
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
@radarhere radarhere merged commit 1457d2c into python-pillow:main Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants