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.

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