-
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
DEV: Use pytest-xdist #2254
DEV: Use pytest-xdist #2254
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2254 +/- ##
=======================================
Coverage 94.44% 94.44%
=======================================
Files 43 43
Lines 7634 7634
Branches 1506 1506
=======================================
Hits 7210 7210
Misses 262 262
Partials 162 162 ☔ View full report in Codecov by Sentry. |
It still seems not to combine the coverage data correctly. I guess the reason is a race-condition between the different python versions. |
This appears to be a naming issue: Looking at https://github.com/py-pdf/pypdf/actions/runs/6516162435/job/17699481914, we have
For https://github.com/py-pdf/pypdf/actions/runs/6523292132/job/17713841210, we get
instead. Possible solution: Add prefix or suffix to the coverage database before uploading the artifact to ensure unique names instead of one common name and thus the files overwriting theirselves. |
@stefan6419846 I think I fixed the issue :-) What do you think about the overall PR? It seems to reduce the CI wall-clock time from ~14min 23s to 9min 20s. |
While in general the randomness source should be sufficient enough, I am not sure whether we can use a more stable approach there (the current Python version as given by the matrix?) or at least ensure that when combining the coverage/downloading the artifacts we actually have the correct number of files. As for the runtime: As it does not have any apparent side effects and it does not change anything relevant on the remaining test setup, this at least is some enhancement. Reducing the runtime by roughly 1/3 while using two parallel executors seems to be a little bit worse than expected, but it does not take into account the pure tests runtime, but the general setup, and thus this might be as good as we can get without adding more parallel executors. |
## What's new ### Security (SEC) - Infinite recursion when using PdfWriter(clone_from=reader) (#2264) by @Alexhuszagh ### New Features (ENH) - Add parameter to select images to be removed (#2214) by @pubpub-zz ### Bug Fixes (BUG) - Correctly handle image mode 1 with FlateDecode (#2249) by @stefan6419846 - Error when filling a value with parentheses #2268 (#2269) by @KanorUbu - Handle empty root outline (#2239) by @pubpub-zz ### Documentation (DOC) - Improve merging docs (#2247) by @stefan6419846 ### Developer Experience (DEV) - Test Python 3.7 with cryptopgraphy provider as well (#2276) by @stefan6419846 - Run CI with windows-latest (#2258) by @MartinThoma - Use pytest-xdist (#2254) by @MartinThoma - Attribute correct authors in the release notes (#2246) by @stefan6419846 ### Maintenance (MAINT) - Apply pre-commit hooks (#2277) by @MartinThoma - Update requirements + mypy fixes (#2275) by @MartinThoma - Explicitly provide Any for IO generic argument (#2272) by @nilehmann ### Testing (TST) - Fix test_image_without_pillow in windows environment (#2257) by @pubpub-zz ### Code Style (STY) - Remove unused import by @MartinThoma [Full Changelog](3.16.4...3.17.0)
Before (example):
After (this PR):
and a second run: