Fix handling of sys.stdout.encoding being None#300
Conversation
Codecov Report
@@ Coverage Diff @@
## master #300 +/- ##
==========================================
+ Coverage 95.39% 95.47% +0.07%
==========================================
Files 20 20
Lines 1043 1060 +17
Branches 103 104 +1
==========================================
+ Hits 995 1012 +17
Misses 27 27
Partials 21 21
Continue to review full report at Codecov.
|
adiroiban
left a comment
There was a problem hiding this comment.
Look good. Only a medium comment regarding the test.
I think that the test can be updated and then merged without another review.
Thanks!
src/towncrier/test/test_check.py
Outdated
| check_call( | ||
| [sys.executable, '-m', 'towncrier.check', "--compare-with", "master"], | ||
| stdout=PIPE, | ||
| ) |
There was a problem hiding this comment.
I don't like how this assertion is done.
From Python docs
Note
Do not use stdout=PIPE or stderr=PIPE with this function. The child process will block if it generates enough output to a pipe to fill up the OS pipe buffer as the pipes are not being read from.
proc = subprocess.Popen([sys.executable, '-m', 'towncrier.check', "--compare-with", "master"], stdout=PIPE, stder=PIPE)
try:
outs, errs = proc.communicate(timeout=15)
except TimeoutExpired:
proc.kill()
outs, errs = proc.communicate()
and then check that err is empty and the out has the expecte output and no sign of error.
Not sure how to check for no errors in stdout... but maybe in the future we can have towncrier sending all errors to stderr and then checking for an empty stderr is enough.
There was a problem hiding this comment.
Implemented without the timeout since py2 doesn't have timeout. Could go for subprocess32 or something if it's particularly important (sure, it's obviously nice).
|
Kick again to see if codecov works better |
Fixes #175.