Repository navigation
Update tests to use test.support.captured_std* #136632
Description
Activity
I'd be ok with those changes but there seem to be many of them. I think it's better if we only change the code if we need to. Now, many recent occurrences in CLI tests are because of my original implementation that I gave as an example so I can take the blame here. I didn't know about this helper and if I did, I would have used it. So for the CLI tests that use
redirect_*, I'm ok with changing them in one go. For the others, especially tests that are very old, I wouldn't bother changing them.Also, to prevent future conflicts, it'd be good to separate the CLI tests into two PR. One PR that touches the CLI tests added in 3.15 only and one PR that touches the CLI tests added in 3.14 so that we can backport it.
Now, before starting any work, I want @serhiy-storchaka's opinion here.
I do not think that there is a reason to switch between
contextlib.redirect_std*andtest.support.captured_std*. The latter is a little simpler, but the difference is only in passing an explicitio.StringIO()argument. This is just a code churn.There is little benefit of changing
test.support.captured_output("std*")totest.support.captured_std*.Only the third group has potential small long-term benefit, because using any of helpers would make the code clearer and easier to read and modify in future. But it has also higher risk of breaking code. This may outweigh the long-term benefit.
I suggest leaving the code as is, but if it is changed in the future, use the standard helper.
That makes sense. Tests can be updated to use the helper as they change over time. I think for the third group, the cases where a helper would have the greatest code readability impact also have higher risk of breaking the test purpose. Closing the sample PR too.
Python's test suite has
test.support.captured_std*utilities to temporarily swapsys.std*with anio.StringIO(). These wrapper functions are used at least 240 times throughout the tests. A few greps1 show quite a few instances where these helpers could be used but are not (45 usecontextlib.redirect_std*, 30 usetest.support.captured_output("std*"), and 15 use a manual solution).Should we update these calls to use the
supportutilities to improve consistency or is it not worth the risk of accidentally changing a test's meaning and cluttering git blame? I found 31d1d72 from last year which did similar updating intest_pydoc/test_pydoc.py. There's also an argument the "update" should go fromtest.support.captured_std*to the newercontextlib.redirect_std*instead, and remove the testing specific utilities.Since this touches a wide range of domains, my thought was to split this up into separate PRs for each test file so the appropriate person can more easily review. But we can also do this in larger chunks, at least for the lower risk changes.
Instances of
contextlib.redirect_std*swappable fortest.support.captured_std*. In some cases allowing the removal ofioandcontextlibimports.test_ast/test_ast.py- L3304, L3348test_compile.py- L971test_compileall.py- L92, L102, L209test_concurrent_futures/test_interpreter_pool.py- L201, L208, L221, L232, L246test_dis.py- L975, L992, L1713, L2432, L2459, L2510, L2544test_httpservers.py- L1321test_pdb.py- L4574test_pickle.py- L732, L760test_platform.py- L761, L769, L809test_profile.py- L96test_pyrepl/test_interact.py- L35, L52, L65, L75, L91, L100, L110, L138, L150, L163, L171test_regrtest.py- L369test_remote_pdb.py- L1526test_sqlite3/util.py- L50test_tokenize.py- L3185, L3212test_uuid.py- L1149, L1182, L1195, L1210, L1224Instances of
test.support.captured_output("std*")swappable for a correspondingtest.support.captured_std*. Low risk to change since the latter calls the former.test_descr.py- L1298test_future_stmt/test_future_multiple_features.py- L15test_itertools.py- L882test_sys.py- L198, L1487test_threading.py- L2058, L2074, L2097, L2137, L2148test_traceback.py- L600, L1817, L1821, L1858, L1871, L1892, L1943, L1983, L2013, L2045, L2117, L2138, L2149, L2159, L3016, L3032, L4817test_warnings/__init__.py- L900, L914, L927, L942Tests with a manual stream save/restore (most are stdin). Higher risk especially when replacing over long blocks.
libregrtest/single.py- L297test_builtin.py- L1748test_capi/check_config.py- L70test_concurrent_futures/test_deadlock.py- L148test_exceptions.py- L71test_faulthandler.py- L399, L806test_fileinput.py- L129, L151test_optparse.py- L160test_profile.py- L90 & L101 callsilentwhich could betest.support.captured_stdoutinstead of reimplementing the function.test_pydoc.py- L478test_unittest/test_runner.py- L1425test_winconsoleio.py- L170test_wsgiref.py- L74Linked PRs
Footnotes
Searched for
captured_output("std,sys.std* =, andredirect_std*so there may be some missing instances. I manually reviewed the ones listed to check if replacement seemed doable (for instancetest_calendar.pyneedsTextIOWrapper(BytesIO)notStringIO, so cannot be easily swapped). ↩