Review: Approve
Wow! Great work. Nice changes to the tests, and thank you for cleaning up the
world.
I have two comments: one each about the most predictably commentable things in
the MP.
(1) For get_real_stdout and friend, would you feel better and be able to remove
that apologetic comment if the code looked like this instead?
___stdout = ___stderr = None
def get_real_stdout():
"""The canonical right place to get __stdout__."""
return ___stdout or sys.__stdout__
def get_real_stderr():
"""The canonical right place to get __stderr__."""
return ___stderr or sys.__stderr__
def set_stdout(new):
"""Set __stdout__ so that you can clean things up later.
Other code can get at the original value with get_real_stdout."""
global ___stdout
___stdout = sys.__stdout__
sys.__stdout__ = new
def set_stderr(new):
"""Set __stderr__ so that you can clean things up later.
Other code can get at the original value with get_real_stderr."""
global ___stderr
___stderr = sys.__stderr__
sys.__stderr__ = new
def reset_stdout():
global ___stdout
result = sys.__stdout__
sys.__stdout__ = ___stdout
___stdout = None
return result
def reset_stderr():
global ___stderr
result = sys.__stderr__
sys.__stderr__ = ___stderr
___stderr = None
return result
Maybe you want those to mess with stdout and stderr too.
Then, in testrunner.options.get_options, instead of directly mucking with
__stderr__...
# If we are running in a subprocess then the test runner code
will
# use stderr as a communication channel back to the spawning
# process so we shouldn't touch it.
if '--resume-layer' not in sys.argv:
sys.__stderr__ = sys.stderr = StringIO()
...we say "zope.testing.testrunner.set_stderr(StringIO())"
Similarly, when we set __stdout__ in doctests, we use the functions above, and
then reset them using the functions.
The advantage is that we are not doing anything at __init__ time, and
everything seems a bit cleaner to me. What do you think?
(2) For the new XXX in src/zope/testing/testrunner/tests.py in which we disable
the tests, please add a bug for the broken tests and mention the number. I
think it's worth following the usual practice.
Thanks again!
--
https://code.launchpad.net/~yellow/zope.testing/fix-tests/+merge/111442
Your team Yellow Squad is requested to review the proposed merge of
lp:~yellow/zope.testing/fix-tests into lp:~launchpad/zope.testing/3.9.4-fork.
--
Mailing list: https://launchpad.net/~yellow
Post to : [email protected]
Unsubscribe : https://launchpad.net/~yellow
More help : https://help.launchpad.net/ListHelp
