gh-148402: add missing stacklevel to warnings.warn() in subprocess.Popen#148400
gh-148402: add missing stacklevel to warnings.warn() in subprocess.Popen#148400stef41 wants to merge 1 commit intopython:mainfrom
Conversation
|
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 |
picnixz
left a comment
There was a problem hiding this comment.
Please add a regression test.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
…ess.Popen Add stacklevel to two warnings.warn() calls in subprocess.Popen so that warnings point to the caller's code instead of subprocess.py internals: - POSIX path (__init__): stacklevel=2 - Windows path (_execute_child): stacklevel=3 (extra frame depth) Add regression tests that assert warning.filename == __file__.
bcc7284 to
8aca8b6
Compare
|
The following commit authors need to sign the Contributor License Agreement: |
| # POSIX | ||
| if pass_fds and not close_fds: | ||
| warnings.warn("pass_fds overriding close_fds.", RuntimeWarning) | ||
| warnings.warn("pass_fds overriding close_fds.", RuntimeWarning, stacklevel=2) |
There was a problem hiding this comment.
Please split the line so it is not exceed 80 columns.
| close_fds=False, pass_fds=(fd, ))) | ||
| self.assertIn('overriding close_fds', str(context.warning)) | ||
|
|
||
| def test_pass_fds_overriding_close_fds_warning_location(self): |
There was a problem hiding this comment.
So this warning was not tested at all?
| @@ -0,0 +1,3 @@ | |||
| Add missing ``stacklevel`` to two :func:`warnings.warn` calls in | |||
There was a problem hiding this comment.
This description contains implementation details not interesting to the user. Just describe what was changed from the user's point of view.
Summary
Fixes #148402.
Both
warnings.warn()calls insubprocess.Popenare missingstacklevel, so warning messages point to internal lines insidesubprocess.pyrather than the caller'sPopen()invocation.Before
After
Changes
__init__(line 915)2warn→__init__→ caller_execute_child(line 1570)3warn→_execute_child→__init__→ callerTests
test_pass_fds_overriding_close_fds_warning_location— assertscontext.filename == __file__test_close_fds_with_stdio— addedcontext.filename == __file__assertionNEWS
Added
Misc/NEWS.d/next/Library/entry for gh-148402.