Skip to content

Commit

Permalink
BUG: Fix regression with run_subprocess (#11230)
Browse files Browse the repository at this point in the history
  • Loading branch information
larsoner authored Oct 7, 2022
1 parent 4a37394 commit 8bd9406
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 13 deletions.
26 changes: 14 additions & 12 deletions mne/utils/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,21 +138,22 @@ def run_subprocess(command, return_code=False, verbose=None, *args, **kwargs):
except Empty:
break
else:
out = out.decode('utf-8')
# Strip newline at end of the string, otherwise we'll end
# up with two subsequent newlines (as the logger adds one)
#
# XXX Once we drop support for Python <3.9, uncomment the
# following line and remove the if/else block below.
#
# out = out.decode('utf-8').removesuffix('\n')
out = out.decode('utf-8')
# log_out = out.removesuffix('\n')
if sys.version_info[:2] >= (3, 9):
out = out.removesuffix('\n')
log_out = out.removesuffix('\n')
elif out.endswith('\n'):
log_out = out[:-1]
else:
if out.endswith('\n'):
out = out[:-1]
log_out = out

logger.info(out)
logger.info(log_out)
all_out += out

while True: # process stderr
Expand All @@ -161,27 +162,28 @@ def run_subprocess(command, return_code=False, verbose=None, *args, **kwargs):
except Empty:
break
else:
err = err.decode('utf-8')
# Strip newline at end of the string, otherwise we'll end
# up with two subsequent newlines (as the logger adds one)
#
# XXX Once we drop support for Python <3.9, uncomment the
# following line and remove the if/else block below.
#
# err = err.decode('utf-8').removesuffix('\n')
err = err.decode('utf-8')
# err_out = err.removesuffix('\n')
if sys.version_info[:2] >= (3, 9):
err = err.removesuffix('\n')
err_out = err.removesuffix('\n')
elif err.endswith('\n'):
err_out = err[:-1]
else:
if err.endswith('\n'):
err = err[:-1]
err_out = err

# Leave this as logger.warning rather than warn(...) to
# mirror the logger.info above for stdout. This function
# is basically just a version of subprocess.call, and
# shouldn't emit Python warnings due to stderr outputs
# (the calling function can check for stderr output and
# emit a warning if it wants).
logger.warning(err)
logger.warning(err_out)
all_err += err

if do_break:
Expand Down
35 changes: 34 additions & 1 deletion mne/utils/tests/test_misc.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import os
import sys

import pytest

import mne
from mne.utils import sizeof_fmt
from mne.utils import sizeof_fmt, run_subprocess, catch_logging


def test_sizeof_fmt():
Expand Down Expand Up @@ -29,3 +32,33 @@ def test_html_repr():
del os.environ[key]
if existing_value is not None:
os.environ[key, existing_value]


@pytest.mark.parametrize('kind', ('stdout', 'stderr'))
def test_run_subprocess(tmp_path, kind):
"""Test run_subprocess."""
fname = tmp_path / 'subp.py'
with open(fname, 'w') as fid:
fid.write(f"""\
import sys
print('foo', file=sys.{kind})
print('bar', file=sys.{kind})
""")
with catch_logging() as log:
stdout, stderr = run_subprocess(
[sys.executable, str(fname)], verbose=True)
log = log.getvalue()
log = '\n'.join(log.split('\n')[1:]) # get rid of header
log = log.replace('\r\n', '\n') # Windows
stdout = stdout.replace('\r\n', '\n')
stderr = stderr.replace('\r\n', '\n')
want = 'foo\nbar\n'
assert log == want
if kind == 'stdout':
std = stdout
other = stderr
else:
std = stderr
other = stdout
assert std == want
assert other == ''

0 comments on commit 8bd9406

Please sign in to comment.