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

gh-37883: Safely skip test_resource file size tests when limits are strict by Shrey-N · Pull Request #145579 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-37883: Safely skip test_resource file size tests when limits are strict - #145579

Merged
encukou merged 20 commits into
python:mainfrom
Shrey-N:fixy-branch
Apr 8, 2026
ConversationCommits (20)ChecksFiles changed
Merged

gh-37883: Safely skip test_resource file size tests when limits are strict#145579
encukou merged 20 commits into
python:mainfrom
Shrey-N:fixy-branch

Conversation

Shrey-N commented Mar 6, 2026 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

This fixes the Standing issue #37883 where test_resource would fail on systems with file size limitations

Previous attempts (like #140872) tried to introduce massive refactoring and helper classes. Based on reviewer feedback I read there, I have tried a minimal fix

  • It checks if the system's hard limit is large enough, and if the OS actually allows the limit to be modified. If not, it uses self.skipTest with an informative message rather than failing with an unhandled OSError or ValueError.
  • It replaces the fragile dual try...finally resource restoration blocks with self.addCleanup(), guaranteeing that limits are reset even during a test assertion failure.

Fixes #37883

bedevere-app Bot added the tests Tests in the Lib/test dir label Mar 6, 2026

python-cla-bot Bot commented Mar 6, 2026 •
edited
Loading

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

bedevere-app Bot mentioned this pull request Mar 6, 2026
Closed

Shrey-N commented Mar 6, 2026

Copy link
Copy Markdown
Contributor Author

I have added a news entry, though I am not sure if it's necessary or for this fix, Please let me know if I should remove it :)

encukou reviewed Mar 10, 2026
View reviewed changes

encukou 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

This looks good!

We don't need a NEWS entry for a test-only change.

Comment thread Lib/test/test_resource.py Outdated
Comment thread Lib/test/test_resource.py Outdated

Shrey-N commented Mar 10, 2026

Copy link
Copy Markdown
Contributor Author

@encukou Alrighty thank you, I was confused as to add it or not so thank you for clearing that up :), also I will incorporate your reviews in a second :)

encukou commented Mar 13, 2026

Copy link
Copy Markdown
Member

I think you can roll back the last 4 commits.

In d21ceb8, the removed comment says:

            # Close will attempt to flush the byte we wrote
            # Restore limit first to avoid getting a spurious error

That should stay, i.e. the try/finally should go inside the with, not the other way around.

Shrey-N commented Mar 13, 2026

Copy link
Copy Markdown
Contributor Author

Alright will look into it asap

Shrey-N commented Mar 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@encukou I believe everything should work now :) Sorry for the late reply

Shrey-N commented Mar 29, 2026

Copy link
Copy Markdown
Contributor Author

@encukou Hi, I believe I fixed the tests that were failing from my side in this PR, I am pretty sure this time, it's not my fault :) lmk if I am to make any other changes :)

encukou reviewed Apr 7, 2026
View reviewed changes
Comment thread Lib/test/test_resource.py
Comment thread Lib/test/test_resource.py
Comment thread Lib/test/test_resource.py Outdated
encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 7, 2026

bedevere-bot commented Apr 7, 2026

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit 4564e1f 🤖

Results will be shown at:

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

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 7, 2026
Shrey-N and others added 3 commits April 7, 2026 20:54
Update based on suggestion
1808a8a 
Co-authored-by: Petr Viktorin <encukou@gmail.com>
Update Lib/test/test_resource.py based on suggestion
6faed32 
Co-authored-by: Petr Viktorin <encukou@gmail.com>
Update Lib/test/test_resource.py
58fb908 
Co-authored-by: Petr Viktorin <encukou@gmail.com>

Shrey-N commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

Accepted all suggestions, thanks!

encukou merged commit 461125a into python:main Apr 8, 2026
87 of 89 checks passed

encukou commented Apr 8, 2026

Copy link
Copy Markdown
Member

Thank you!

encukou added needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels Apr 8, 2026

miss-islington-app Bot commented Apr 8, 2026

Copy link
Copy Markdown

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

miss-islington-app Bot commented Apr 8, 2026

Copy link
Copy Markdown

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

miss-islington-app Bot commented Apr 8, 2026

Copy link
Copy Markdown

Sorry, @Shrey-N and @encukou, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 461125aaa331fe2b39452c852ee26d0161b2436e 3.13

miss-islington-app Bot commented Apr 8, 2026

Copy link
Copy Markdown

Sorry, @Shrey-N and @encukou, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 461125aaa331fe2b39452c852ee26d0161b2436e 3.14

encukou removed needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels Apr 8, 2026

encukou commented Apr 8, 2026

Copy link
Copy Markdown
Member

We usually backport test fixes to earlier versions, but mostly to avoid conflicts in future fixes. Since there are conflicts already, I'll skip backporting.

If you need the fix in an earlier version, feel free to open a backport PR.

ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
pythongh-37883: Safely skip test_resource file size tests when limits…
cc4c17c 
… are strict (pythonGH-145579)
aisk mentioned this pull request Oct 14, 2026
Merged
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

encukou encukou left review comments

Assignees

encukou

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test_resource fails when file size is limited

3 participants

Back | FazBrowse Home | New Git URL