fix: correct error message for stdout pipe check in run_process#3416
fix: correct error message for stdout pipe check in run_process#3416jakkdl merged 8 commits intopython-trio:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3416 +/- ##
===============================================
Coverage 100.00000% 100.00000%
===============================================
Files 128 128
Lines 19417 19418 +1
Branches 1318 1318
===============================================
+ Hits 19417 19418 +1
🚀 New features to boost your workflow:
|
|
You forgot to update the tests mate |
|
Good catch @CoolCat467! Updated the test to match the corrected error message — the stdin pipe check now expects |
|
Please fix the news fragment syntax! |
src/trio/_tests/test_subprocess.py
Outdated
| pipe_stdin_error = r"^stdin=subprocess\.PIPE is only valid with nursery\.start, since that's the only way to access the pipe(; use nursery\.start or pass the data you want to write directly)*$" | ||
| with pytest.raises(ValueError, match=pipe_stdin_error): | ||
| await run_process(CAT, stdin=subprocess.PIPE) | ||
| pipe_stdout_error = r"^stdout=subprocess\.PIPE is only valid with nursery\.start, since that's the only way to access the pipe(; use nursery\.start or pass the data you want to write directly)*$" |
There was a problem hiding this comment.
The last part uses a * but I think the regex can be more specific.
(Same comment applies for the one above, too)
|
Fixed — switched to double backticks ( |
|
Please address the review too! (and you don't need to write out a comment acknowledging what you finished; the commits have that data.) |
A5rocks
left a comment
There was a problem hiding this comment.
Sorry, I was thinking something like this. What do you think?
A5rocks
left a comment
There was a problem hiding this comment.
Thanks! I'm not sure why pre-commit is erroring on this PR... The error message doesn't make sense, because your branch doesn't have either of the listed versions!
Hopefully just pulling in main works :/
|
Hey @nightcityblade, it looks like that was the first time we merged one of your PRs! Thanks so much! 🎉 🎂 If you want to keep contributing, we'd love to have you. So, I just sent you an invitation to join the python-trio organization on Github! If you accept, then here's what will happen:
If you want to read more, here's the relevant section in our contributing guide. Alternatively, you're free to decline or ignore the invitation. You'll still be able to contribute as much or as little as you like, and I won't hassle you about joining again. But if you ever change your mind, just let us know and we'll send another invitation. We'd love to have you, but more importantly we want you to do whatever's best for you. If you have any questions, well... I am just a humble Python script, so I probably can't help. But please do post a comment here, or in our chat, or on our forum, whatever's easiest, and someone will help you out! |
Fixes #3409
The error message for the
stdout=subprocess.PIPEcheck inrun_processincorrectly said "stdin=subprocess.PIPE". This fixes it to say "stdout=subprocess.PIPE".This is a recreation of #3398 with the same fix plus a newsfragment.