Avoid interactive pdb when pytest tests itself - fix #2023 #2580
Conversation
6843385 to
f53a7c0
Compare
|
And you can add the EP2017 sprint tag to this pull request 😉 |
f53a7c0 to
e5853ff
Compare
e5853ff to
f588238
Compare
|
@RonnyPfannschmidt would you like to take a better look or can we merge this? |
| return subprocess.Popen(cmdargs, | ||
| stdout=stdout, stderr=stderr, **kw) | ||
|
|
||
| popen = subprocess.Popen(cmdargs, stdin=subprocess.PIPE, stdout=stdout, stderr=stderr, **kw) |
There was a problem hiding this comment.
why PIPE when the message says DEVNULL ?
There was a problem hiding this comment.
Oh, you are right. The original solution was subprocess.DEVNULL as stdin, but this was not available in older python version. Therefore I replaced it with a closed pipe, but I forgot to change the commit message too.
There was a problem hiding this comment.
nitpick, please add a comment to switch to DEVNULL once we drop python2.6 support
There was a problem hiding this comment.
subprocess.DEVNULL only was added in 3.3, but you can open os.devnull (which is a filename) before that.
There was a problem hiding this comment.
@RonnyPfannschmidt, I think, the final solution with the closed pipe is much better than the original, because this is tinier.
In other hands, the DEVNULL solution opened an unnecessary fd, but we wanted to make a not interactive pdb only.
f588238 to
7d8ff55
Compare
The debugging.py calls post_mortem() on error and pdb will drops an interactive debugger when the stdin is a readable fd.
7d8ff55 to
50764d9
Compare
|
I updated the changelog and rebased to upstream master. |
|
If anybody else needs more time to take a further look let me know, otherwise I plan to merge this tomorrow. |
|
Thanks again @andras-tim! |
The debugging.py calls post_mortem() on error and pdb will drops an
interactive debugger when the stdin is a readable fd.
Thanks for submitting a PR, your contribution is really appreciated!
Here's a quick checklist that should be present in PRs:
$issue_id.$typefor example (588.bug)removal,feature,bugfix,vendor,docortrivialbugfix,vendor,docortrivialfixes, targetmaster; for removals or features targetfeatures;Unless your change is a trivial or a documentation fix (e.g., a typo or reword of a small section) please:
AUTHORS;