Skip to content

gh-148402: add missing stacklevel to warnings.warn() in subprocess.Popen#148668

Open
stef41 wants to merge 1 commit intopython:mainfrom
stef41:fix-subprocess-missing-stacklevel
Open

gh-148402: add missing stacklevel to warnings.warn() in subprocess.Popen#148668
stef41 wants to merge 1 commit intopython:mainfrom
stef41:fix-subprocess-missing-stacklevel

Conversation

@stef41
Copy link
Copy Markdown

@stef41 stef41 commented Apr 17, 2026

Summary

Add stacklevel to two warnings.warn() calls in subprocess.Popen so
that warnings point to the caller's code instead of subprocess internals.

Changes

Location Warning stacklevel Reason
__init__ (POSIX) pass_fds overriding close_fds 2 caller → __init__ → warn
_execute_child (Windows) handle_list overriding close_fds 3 caller → __init__ → _execute_child → warn

Tests

  • New test_pass_fds_overriding_close_fds_warning_location in POSIXProcessTestCase: calls Popen() directly and asserts cm.filename == __file__.
  • Enhanced test_close_fds_with_stdio in Win32ProcessTestCase: switched from check_warnings to assertWarns, added cm.filename == __file__ assertion.

Supersedes #148400 which was accidentally closed during a force push.

…ess.Popen

Add stacklevel to two warnings.warn() calls in subprocess.Popen so that
warnings point to the caller's code instead of subprocess internals:

- POSIX: 'pass_fds overriding close_fds' in __init__: stacklevel=2
- Windows: 'handle_list overriding close_fds' in _execute_child: stacklevel=3
  (_execute_child is called from __init__, adding one extra frame)
@stef41 stef41 requested a review from gpshead as a code owner April 17, 2026 08:36
@python-cla-bot
Copy link
Copy Markdown

python-cla-bot bot commented Apr 17, 2026

All commit authors signed the Contributor License Agreement.

CLA signed

close_fds=False, pass_fds=(fd, )))
self.assertIn('overriding close_fds', str(context.warning))

def test_pass_fds_overriding_close_fds_warning_location(self):
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.

2. The warning message was already tested in test_pass_fds (line 3174: assertIn('overriding close_fds', str(context.warning))).

If the warning message was already tested, why not simply add an assertion for filename there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants