Skip to content

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

gh-152190: Fix memory checking failure in test_zipfile64.py#152203
gpshead merged 7 commits into
python:mainfrom
danny0838:gh-51067-6

Conversation

@danny0838

@danny0838 danny0838 commented Jun 25, 2026

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 bedevere-app Bot added the tests Tests in the Lib/test dir label Jun 25, 2026
@StanFromIreland 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
danny0838 force-pushed the gh-51067-6 branch 3 times, most recently from c98f30e to 34a009b Compare June 25, 2026 14:52
@danny0838
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.
@danny0838

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.

@StanFromIreland

Copy link
Copy Markdown
Member

!buildbot bigmem

@bedevere-bot

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 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 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.

nice refactoring

@gpshead
gpshead merged commit e5ed2ad into python:main Aug 24, 2026
51 checks passed
@gpshead gpshead added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Aug 24, 2026
@miss-islington-app

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.
🐍🍒⛏🤖

@miss-islington-app

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.
🐍🍒⛏🤖

@miss-islington-app

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

@miss-islington-app

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.
🐍🍒⛏🤖

@miss-islington-app

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

@miss-islington-app

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 gpshead removed needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Aug 24, 2026
@gpshead

gpshead commented Aug 24, 2026

Copy link
Copy Markdown
Member

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

@danny0838

danny0838 commented Aug 24, 2026

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>
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