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

Convert some tests to pytest style #4369

Merged
merged 1 commit into from
Jan 26, 2020
Merged

Convert some tests to pytest style #4369

merged 1 commit into from
Jan 26, 2020

Conversation

jdufresne
Copy link
Contributor

To better follow conventional pytest style, this removes the outer
wrapper class in favor of a function for some tests. These tests were
picked as they are relatively simple and presented no barriers to a
quick port. The assert* methods are replaced with assert statements.
When necessary, a fixture is used to create a temporary directory.

This commit does not convert the entire test suite to this style as some
test classes use methods or other advanced features that are difficult
to automatically convert. The goal is to address these issues in
followup commits.

Refs #4193


To reviewers, I'm happy expand or shrink the scope of this change as desired. If there is a preference for this to be split up over multiple commits, let me know. I'm happy to make adjustments.

To better follow conventional pytest style, this removes the outer
wrapper class in favor of a function for some tests. These tests were
picked as they are relatively simple and presented no barriers to a
quick port. The assert* methods are replaced with assert statements.
When necessary, a fixture is used to create a temporary directory.

This commit does not convert the entire test suite to this style as some
test classes use methods or other advanced features that are difficult
to automatically convert. The goal is to address these issues in
followup commits.

Refs #4193
im.load()


def test_save(tmp_path):
Copy link
Member

Choose a reason for hiding this comment

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

Noting something new to me, this looks handy:

The tmp_path fixture

https://docs.pytest.org/en/latest/tmpdir.html

@hugovk
Copy link
Member

hugovk commented Jan 20, 2020

I'm fine with this. Any comments from anyone else?


I tried timing running the changed tests in this commit (using pytest) vs. the previous commit (using unittest), and the numbers were more or less inconclusive.

  • 3m49s pytest
  • 3m58s unittest
  • 3m41s unittest
  • 3m47s unittest
  • 4m04s pytest
  • 4m05s pytest
  • 4m05s unittest
  • 4m56s unittest
for i in {1..100}; do pytest Tests/test_000_sanity.py Tests/test_binary.py Tests/test_box_blur.py Tests/test_core_resources.py Tests/test_file_bufrstub.py Tests/test_file_container.py Tests/test_file_gd.py Tests/test_file_gimppalette.py Tests/test_file_wal.py Tests/test_format_lab.py Tests/test_image_getbands.py Tests/test_image_getbbox.py Tests/test_image_getim.py Tests/test_image_getpalette.py Tests/test_image_putpalette.py Tests/test_image_tobytes.py Tests/test_imagestat.py Tests/test_locale.py Tests/test_main.py Tests/test_util.py; done

@hugovk hugovk merged commit 22a6738 into python-pillow:master Jan 26, 2020
@hugovk
Copy link
Member

hugovk commented Jan 26, 2020

Alright, let's go ahead with this. Thanks!

@jdufresne jdufresne deleted the pytest branch January 26, 2020 21:25
This pull request was closed.
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.

2 participants