-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
TST: simplify test_get_images #2668
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2668 +/- ##
=======================================
Coverage 95.00% 95.00%
=======================================
Files 50 50
Lines 8356 8356
Branches 1673 1673
=======================================
Hits 7939 7939
Misses 259 259
Partials 158 158 ☔ View full report in Codecov by Sentry. |
This may be expected to check that no exception are raised
Have not been able to get through I've rerun the checks to confirm coverage and your PR has reduced the test coverage. Can you please revisit it ? |
@pubpub-zz Thanks for the quick feedback! I have added a new test to test_page.py so to cover the |
to prevent writing you may perhaps have a look at test_corrupted_jpeg_iss2266 in test_images.py |
@pubpub-zz Would you like to check for similarity as in |
Do we really need to write to the file system? Shouldn't writing to |
@pubpub-zz, @stefan6419846 I might be wrong, but since the second assert has already the manipulations with BytesIO, is there anything still missing, please? Let me know if I am misunderstanding something - for now, I have removed the writing to file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out of tmp_path, coverage looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience.
Fixes #2428
Imho, the test does too many things. Thus, instead of replacing with e.g.
txt_file_path fixture
, I removed things:tmp_path