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

gh-156570: handle lazy warnings import in test helper by mikemikimike · Pull Request #156573 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-156570: handle lazy warnings import in test helper - #156573

Closed
mikemikimike wants to merge 5 commits into
python:mainfrom
mikemikimike:fix/warnings-helper-lazy-imports
ConversationCommits (5)ChecksFiles changed
Closed

gh-156570: handle lazy warnings import in test helper#156573
mikemikimike wants to merge 5 commits into
python:mainfrom
mikemikimike:fix/warnings-helper-lazy-imports

Conversation

mikemikimike commented Aug 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Issue: #156570

Summary

warnings_helper._filterwarnings() indexed sys.modules['warnings'] directly. Under -X lazy_imports=all, evaluating the module-level import can leave that entry unresolved, causing check_warnings() to raise KeyError.

Use the already imported module as a fallback when the warnings entry is absent, and add regression coverage for this state.

Testing

  • python Lib/test/test_warnings_helper.py
  • python -m py_compile Lib/test/support/warnings_helper.py Lib/test/test_warnings_helper.py
  • git diff --check

Compatibility

The change preserves the existing module replacement behavior used by test_warnings while avoiding a KeyError when lazy imports have not populated sys.modules.

AI assistance was used to identify the issue and prepare this patch.

Fixes #156570

python-cla-bot Bot commented Aug 29, 2026 •
edited
Loading

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

bedevere-app Bot commented Aug 29, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

bedevere-app Bot mentioned this pull request Aug 29, 2026
Open

bedevere-app Bot commented Aug 29, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

StanFromIreland reviewed Aug 30, 2026
View reviewed changes
Comment thread Lib/test/test_warnings_helper.py Outdated

mikemikimike commented Aug 30, 2026

Copy link
Copy Markdown
Author

Thanks for the review. I moved the regression test into the existing Lib/test/test_support.py test suite and removed the standalone test file. The test preserves the module-replacement scenario and restores the warning state afterwards. Verified with PCbuild/amd64/python_d.exe -m test test_support -v (72 tests passed, 4 skipped).

mikemikimike commented Aug 31, 2026

Copy link
Copy Markdown
Author

The change does not add a new test helper: the regression is added to the existing Lib/test/test_support.py alongside the related check_warnings tests. The only new file is the standard Misc/NEWS.d entry required for the bug fix. Closing this clarification thread.

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

StanFromIreland StanFromIreland left review comments

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test.support.warnings_helper.check_warnings() raises KeyError: 'warnings' under -X lazy_imports=all

2 participants

Back | FazBrowse Home | New Git URL