Skip to content

Commit

Permalink
Fix a couple spinner edge cases.
Browse files Browse the repository at this point in the history
  • Loading branch information
cjerdonek committed Mar 3, 2019
1 parent ffefa91 commit fe79372
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 9 deletions.
3 changes: 3 additions & 0 deletions news/6312.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
The spinner no longer displays a completion message after subprocess calls
not needing a spinner. It also no longer incorrectly reports an error after
certain subprocess calls to Git that succeeded.
15 changes: 11 additions & 4 deletions src/pip/_internal/utils/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,10 @@ def call_subprocess(
stdout = None
else:
stdout = subprocess.PIPE

# Only use the spinner when we're capturing stdout and we have one.
use_spinner = not show_stdout and spinner is not None

if command_desc is None:
command_desc = format_command_args(cmd)

Expand Down Expand Up @@ -738,19 +742,22 @@ def call_subprocess(
logger.debug(line)
else:
# Update the spinner
if spinner is not None:
if use_spinner:
spinner.spin()
try:
proc.wait()
finally:
if proc.stdout:
proc.stdout.close()
if spinner is not None:
if proc.returncode:
proc_had_error = (
proc.returncode and proc.returncode not in extra_ok_returncodes
)
if use_spinner:
if proc_had_error:
spinner.finish("error")
else:
spinner.finish("done")
if proc.returncode and proc.returncode not in extra_ok_returncodes:
if proc_had_error:
if on_returncode == 'raise':
if (logger.getEffectiveLevel() > std_logging.DEBUG and
not show_stdout):
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ def test_info_logging_with_show_stdout_true(self, capfd, caplog):
# output is already being written to the console.
self.check_result(
capfd, caplog, log_level, spinner, result, expected,
expected_spinner=(0, 'done'),
expected_spinner=(0, None),
)

@pytest.mark.parametrize((
Expand All @@ -936,13 +936,13 @@ def test_info_logging_with_show_stdout_true(self, capfd, caplog):
# Test some cases that should result in show_spinner false.
(0, False, None, logging.DEBUG, (None, 'done', 0)),
# Test show_stdout=True.
(0, True, None, logging.DEBUG, (None, 'done', 0)),
(0, True, None, logging.INFO, (None, 'done', 0)),
(0, True, None, logging.WARNING, (None, 'done', 0)),
(0, True, None, logging.DEBUG, (None, None, 0)),
(0, True, None, logging.INFO, (None, None, 0)),
(0, True, None, logging.WARNING, (None, None, 0)),
# Test a non-zero exit status.
(3, False, None, logging.INFO, (InstallationError, 'error', 2)),
# Test a non-zero exit status also in extra_ok_returncodes.
(3, False, (3, ), logging.INFO, (None, 'error', 2)),
(3, False, (3, ), logging.INFO, (None, 'done', 2)),
])
def test_spinner_finish(
self, exit_status, show_stdout, extra_ok_returncodes, log_level,
Expand Down

0 comments on commit fe79372

Please sign in to comment.