FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-152190: Fix memory checking failure in `test_zipfile64.py` by danny0838 · Pull Request #152203 · python/cpython · GitHub

Repository navigation

gh-152190: Fix memory checking failure in test_zipfile64.py - #152203

Merged
gpshead merged 7 commits into
python:mainfrom
danny0838:gh-51067-6
Aug 24, 2026
Merged

gpshead merged 7 commits into
python:mainfrom
danny0838:gh-51067-6

Conversation

danny0838 commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Remove the overly restrictive allowed_memory override (200 KiB) in test_strip_removed_large_file_with_dd_no_sig to prevent a memory checking failure.

Additionally, revise the comment about the empirical 10 MiB memory threshold, optimize the code, and clear improper comments.

bedevere-app Bot added the tests Tests in the Lib/test dir label Jun 25, 2026
StanFromIreland changed the title gh-51067: Fix memory checking failure in test_strip_removed_large_file_with_dd_no_sig gh-152190: Fix memory checking failure in test_strip_removed_large_file_with_dd_no_sig Jun 25, 2026
danny0838 force-pushed the gh-51067-6 branch 3 times, most recently from c98f30e to 34a009b Compare June 25, 2026 14:52
danny0838 marked this pull request as draft June 25, 2026 17:38
…arge_file_with_dd_no_sig`

Remove the overly restrictive `allowed_memory` override (200 KiB) in
`test_strip_removed_large_file_with_dd_no_sig` to prevent a memory
checking failure.
Introduce the `assert_memory_usage` context manager to narrow the scope
of tracemalloc tracking down exclusively to the file-repacking phase.

This prevents previous file-writing side effects from interfering with
the baseline, improves tracing accuracy, and eliminates redundant inner
sub-function wrappers.

Copy link
Copy Markdown
Contributor Author

It seems that a dynamic memory threshold cannot be easily calculated. Keep the original empirical threshold and revise the comment to be more clear.

Additionally, introduce the assert_memory_usage context manager and refactor the test code to improve memory checking accuracy as well as remove redundant inner sub-function wrappers.

Copy link
Copy Markdown
Member

!buildbot bigmem

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @StanFromIreland for commit 5335be0 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F152203%2Fmerge

The command will test the builders whose names match following regular expression: bigmem

The builders matched are:

  • aarch64 Ubuntu 24.04 BigMem PR

danny0838 changed the title gh-152190: Fix memory checking failure in test_strip_removed_large_file_with_dd_no_sig gh-152190: Fix memory checking failure in test_zipfile64.py Jun 26, 2026
Remove redundant "TESTFN2" disk space warnings from TestRepack, as
these tests exclusively use TemporaryFile().

Additionally, remove the repetitive `self.assertFalse(f.closed)` checks
from `TestRepack` methods since it's already verified in
`TestsWithSourceFile`.
Rename `TestRepack` to `TestRepacker` to better reflect its coverage of
`zipfile._Repacker`.

Move heavy text chunk generation and common constants from `setUp`
to `setUpClass` to ensure single initialization across tests.

Clean up repetitive local definitions of filenames and lorem text by
promoting them to class properties.

Reduce the test file size from 8 GiB to 4.1 GiB, which is large enough
to trigger ZIP64 extension and sufficient for the testing purpose.

gpshead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

nice refactoring

gpshead merged commit e5ed2ad into python:main Aug 24, 2026
51 checks passed
gpshead added needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Aug 24, 2026

Copy link
Copy Markdown

Thanks @danny0838 for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

Copy link
Copy Markdown

Thanks @danny0838 for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

Copy link
Copy Markdown

Sorry, @danny0838 and @gpshead, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker e5ed2ad9be8e0014a3bf4dc9f89c4ad2695500cd 3.13

Copy link
Copy Markdown

Thanks @danny0838 for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖

Copy link
Copy Markdown

Sorry, @danny0838 and @gpshead, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker e5ed2ad9be8e0014a3bf4dc9f89c4ad2695500cd 3.14

Copy link
Copy Markdown

Sorry, @danny0838 and @gpshead, I could not cleanly backport this to 3.15 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker e5ed2ad9be8e0014a3bf4dc9f89c4ad2695500cd 3.15

gpshead removed needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Aug 24, 2026

gpshead commented Aug 24, 2026

Copy link
Copy Markdown
Member

(this was for a recent feature, no backports needed)

danny0838 commented Aug 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@gpshead Can you help check other remove/repack related PRs? There are at least 3, listed in the README page of the zipremove package as badges.

clin1234 pushed a commit to clin1234/cpython that referenced this pull request Aug 27, 2026
…ythonGH-152203)

* pythongh-152190: Fix memory checking failure in `test_strip_removed_large_file_with_dd_no_sig`

Remove the overly restrictive `allowed_memory` override (200 KiB) in
`test_strip_removed_large_file_with_dd_no_sig` to prevent a memory
checking failure.

* pythongh-152190: Revise comment about the empirical memory threshold

* pythongh-152190: Improve memory checking accuracy for `test_zipfile64`

Introduce the `assert_memory_usage` context manager to narrow the scope
of tracemalloc tracking down exclusively to the file-repacking phase.

This prevents previous file-writing side effects from interfering with
the baseline, improves tracing accuracy, and eliminates redundant inner
sub-function wrappers.

* pythongh-152190: Improve coding style and docstrings

* pythongh-152190: Remove unneeded comments and checks

Remove redundant "TESTFN2" disk space warnings from TestRepack, as
these tests exclusively use TemporaryFile().

Additionally, remove the repetitive `self.assertFalse(f.closed)` checks
from `TestRepack` methods since it's already verified in
`TestsWithSourceFile`.

* pythongh-152190: Further optimize tests and tidy code

Rename `TestRepack` to `TestRepacker` to better reflect its coverage of
`zipfile._Repacker`.

Move heavy text chunk generation and common constants from `setUp`
to `setUpClass` to ensure single initialization across tests.

Clean up repetitive local definitions of filenames and lorem text by
promoting them to class properties.

Reduce the test file size from 8 GiB to 4.1 GiB, which is large enough
to trigger ZIP64 extension and sufficient for the testing purpose.

---------

Co-authored-by: Zachary Ware <zach@python.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL