Skip to content

Comments

Fix error message: says stdout but checks stdin#3398

Open
bysiber wants to merge 1 commit intopython-trio:mainfrom
bysiber:fix/subprocess-stdin-pipe-error-message
Open

Fix error message: says stdout but checks stdin#3398
bysiber wants to merge 1 commit intopython-trio:mainfrom
bysiber:fix/subprocess-stdin-pipe-error-message

Conversation

@bysiber
Copy link

@bysiber bysiber commented Feb 20, 2026

Problem

In _run_process(), when stdin=subprocess.PIPE is passed without using nursery.start, the error message incorrectly says:

stdout=subprocess.PIPE is only valid with nursery.start...

The condition checks stdin is subprocess.PIPE, but the message refers to stdout. This sends users looking at the wrong argument.

Fix

Changed "stdout=subprocess.PIPE" to "stdin=subprocess.PIPE" in the error message so it matches the actual condition being checked.

@codecov
Copy link

codecov bot commented Feb 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.91763%. Comparing base (2fd138e) to head (3972342).

❌ Your project check has failed because the head coverage (99.91763%) is below the target coverage (100.00000%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@                 Coverage Diff                  @@
##                 main       #3398         +/-   ##
====================================================
- Coverage   100.00000%   99.91763%   -0.08237%     
====================================================
  Files             128         128                 
  Lines           19424       19424                 
  Branches         1318        1318                 
====================================================
- Hits            19424       19408         -16     
- Misses              0          12         +12     
- Partials            0           4          +4     
Files with missing lines Coverage Δ
src/trio/_subprocess.py 96.34703% <ø> (-3.65297%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The ValueError raised when stdin=subprocess.PIPE is used without
nursery.start incorrectly says 'stdout=subprocess.PIPE'.  This is
confusing for users since the actual problem is with the stdin
argument, not stdout.
@bysiber bysiber force-pushed the fix/subprocess-stdin-pipe-error-message branch from 62e1a78 to 3972342 Compare February 20, 2026 06:55
Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Please fix the tests and add a newsfragment.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants