Browse Source

Fix regression in which "check" action errored on certain systems (#597, #598).

Dan Helfman 2 years ago
parent
commit
9c42e7e817

+ 4 - 2
NEWS

@@ -1,5 +1,7 @@
-1.7.4.dev0
- * #596: Fix special file detection when broken symlinks are encountered.
+1.7.4
+ * #596: Fix special file detection erroring when broken symlinks are encountered.
+ * #597, #598: Fix regression in which "check" action errored on certain systems ("Cannot determine
+   Borg repository ID").
 
 1.7.3
  * #357: Add "break-lock" action for removing any repository and cache locks leftover from Borg

+ 21 - 13
borgmatic/borg/create.py

@@ -7,7 +7,12 @@ import stat
 import tempfile
 
 from borgmatic.borg import environment, feature, flags, state
-from borgmatic.execute import DO_NOT_CAPTURE, execute_command, execute_command_with_processes
+from borgmatic.execute import (
+    DO_NOT_CAPTURE,
+    execute_command,
+    execute_command_and_capture_output,
+    execute_command_with_processes,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -259,10 +264,9 @@ def collect_special_file_paths(
     Borg would encounter during a create. These are all paths that could cause Borg to hang if its
     --read-special flag is used.
     '''
-    paths_output = execute_command(
+    paths_output = execute_command_and_capture_output(
         create_command + ('--dry-run', '--list'),
-        output_log_level=None,
-        borg_local_path=local_path,
+        capture_stderr=True,
         working_directory=working_directory,
         extra_environment=borg_environment,
     )
@@ -456,12 +460,16 @@ def create_archive(
             working_directory=working_directory,
             extra_environment=borg_environment,
         )
-
-    return execute_command(
-        create_command,
-        output_log_level,
-        output_file,
-        borg_local_path=local_path,
-        working_directory=working_directory,
-        extra_environment=borg_environment,
-    )
+    elif output_log_level is None:
+        return execute_command_and_capture_output(
+            create_command, working_directory=working_directory, extra_environment=borg_environment,
+        )
+    else:
+        execute_command(
+            create_command,
+            output_log_level,
+            output_file,
+            borg_local_path=local_path,
+            working_directory=working_directory,
+            extra_environment=borg_environment,
+        )

+ 12 - 7
borgmatic/borg/info.py

@@ -1,7 +1,7 @@
 import logging
 
 from borgmatic.borg import environment, feature, flags
-from borgmatic.execute import execute_command
+from borgmatic.execute import execute_command, execute_command_and_capture_output
 
 logger = logging.getLogger(__name__)
 
@@ -55,9 +55,14 @@ def display_archives_info(
         )
     )
 
-    return execute_command(
-        full_command,
-        output_log_level=None if info_arguments.json else logging.WARNING,
-        borg_local_path=local_path,
-        extra_environment=environment.make_environment(storage_config),
-    )
+    if info_arguments.json:
+        return execute_command_and_capture_output(
+            full_command, extra_environment=environment.make_environment(storage_config),
+        )
+    else:
+        execute_command(
+            full_command,
+            output_log_level=logging.WARNING,
+            borg_local_path=local_path,
+            extra_environment=environment.make_environment(storage_config),
+        )

+ 2 - 4
borgmatic/borg/list.py

@@ -4,7 +4,7 @@ import logging
 import re
 
 from borgmatic.borg import environment, feature, flags, rlist
-from borgmatic.execute import execute_command
+from borgmatic.execute import execute_command, execute_command_and_capture_output
 
 logger = logging.getLogger(__name__)
 
@@ -151,7 +151,7 @@ def list_archive(
 
         # Ask Borg to list archives. Capture its output for use below.
         archive_lines = tuple(
-            execute_command(
+            execute_command_and_capture_output(
                 rlist.make_rlist_command(
                     repository,
                     storage_config,
@@ -160,8 +160,6 @@ def list_archive(
                     local_path,
                     remote_path,
                 ),
-                output_log_level=None,
-                borg_local_path=local_path,
                 extra_environment=borg_environment,
             )
             .strip('\n')

+ 14 - 7
borgmatic/borg/rinfo.py

@@ -1,7 +1,7 @@
 import logging
 
 from borgmatic.borg import environment, feature, flags
-from borgmatic.execute import execute_command
+from borgmatic.execute import execute_command, execute_command_and_capture_output
 
 logger = logging.getLogger(__name__)
 
@@ -44,9 +44,16 @@ def display_repository_info(
         + flags.make_repository_flags(repository, local_borg_version)
     )
 
-    return execute_command(
-        full_command,
-        output_log_level=None if rinfo_arguments.json else logging.WARNING,
-        borg_local_path=local_path,
-        extra_environment=environment.make_environment(storage_config),
-    )
+    extra_environment = environment.make_environment(storage_config)
+
+    if rinfo_arguments.json:
+        return execute_command_and_capture_output(
+            full_command, extra_environment=extra_environment,
+        )
+    else:
+        execute_command(
+            full_command,
+            output_log_level=logging.WARNING,
+            borg_local_path=local_path,
+            extra_environment=extra_environment,
+        )

+ 11 - 14
borgmatic/borg/rlist.py

@@ -1,7 +1,7 @@
 import logging
 
 from borgmatic.borg import environment, feature, flags
-from borgmatic.execute import execute_command
+from borgmatic.execute import execute_command, execute_command_and_capture_output
 
 logger = logging.getLogger(__name__)
 
@@ -33,11 +33,8 @@ def resolve_archive_name(
         + flags.make_repository_flags(repository, local_borg_version)
     )
 
-    output = execute_command(
-        full_command,
-        output_log_level=None,
-        borg_local_path=local_path,
-        extra_environment=environment.make_environment(storage_config),
+    output = execute_command_and_capture_output(
+        full_command, extra_environment=environment.make_environment(storage_config),
     )
     try:
         latest_archive = output.strip().splitlines()[-1]
@@ -117,12 +114,12 @@ def list_repository(
         repository, storage_config, local_borg_version, rlist_arguments, local_path, remote_path
     )
 
-    output = execute_command(
-        main_command,
-        output_log_level=None if rlist_arguments.json else logging.WARNING,
-        borg_local_path=local_path,
-        extra_environment=borg_environment,
-    )
-
     if rlist_arguments.json:
-        return output
+        return execute_command_and_capture_output(main_command, extra_environment=borg_environment,)
+    else:
+        execute_command(
+            main_command,
+            output_log_level=logging.WARNING,
+            borg_local_path=local_path,
+            extra_environment=borg_environment,
+        )

+ 3 - 6
borgmatic/borg/version.py

@@ -1,7 +1,7 @@
 import logging
 
 from borgmatic.borg import environment
-from borgmatic.execute import execute_command
+from borgmatic.execute import execute_command_and_capture_output
 
 logger = logging.getLogger(__name__)
 
@@ -18,11 +18,8 @@ def local_borg_version(storage_config, local_path='borg'):
         + (('--info',) if logger.getEffectiveLevel() == logging.INFO else ())
         + (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ())
     )
-    output = execute_command(
-        full_command,
-        output_log_level=None,
-        borg_local_path=local_path,
-        extra_environment=environment.make_environment(storage_config),
+    output = execute_command_and_capture_output(
+        full_command, extra_environment=environment.make_environment(storage_config),
     )
 
     try:

+ 36 - 16
borgmatic/execute.py

@@ -147,7 +147,7 @@ def log_outputs(processes, exclude_stdouts, output_log_level, borg_local_path):
         }
 
 
-def log_command(full_command, input_file, output_file):
+def log_command(full_command, input_file=None, output_file=None):
     '''
     Log the given command (a sequence of command/argument strings), along with its input/output file
     paths.
@@ -178,15 +178,14 @@ def execute_command(
 ):
     '''
     Execute the given command (a sequence of command/argument strings) and log its output at the
-    given log level. If output log level is None, instead capture and return the output. (Implies
-    run_to_completion.) If an open output file object is given, then write stdout to the file and
-    only log stderr (but only if an output log level is set). If an open input file object is given,
-    then read stdin from the file. If shell is True, execute the command within a shell. If an extra
-    environment dict is given, then use it to augment the current environment, and pass the result
-    into the command. If a working directory is given, use that as the present working directory
-    when running the command. If a Borg local path is given, and the command matches it (regardless
-    of arguments), treat exit code 1 as a warning instead of an error. If run to completion is
-    False, then return the process for the command without executing it to completion.
+    given log level. If an open output file object is given, then write stdout to the file and only
+    log stderr. If an open input file object is given, then read stdin from the file. If shell is
+    True, execute the command within a shell. If an extra environment dict is given, then use it to
+    augment the current environment, and pass the result into the command. If a working directory is
+    given, use that as the present working directory when running the command. If a Borg local path
+    is given, and the command matches it (regardless of arguments), treat exit code 1 as a warning
+    instead of an error. If run to completion is False, then return the process for the command
+    without executing it to completion.
 
     Raise subprocesses.CalledProcessError if an error occurs while running the command.
     '''
@@ -195,12 +194,6 @@ def execute_command(
     do_not_capture = bool(output_file is DO_NOT_CAPTURE)
     command = ' '.join(full_command) if shell else full_command
 
-    if output_log_level is None:
-        output = subprocess.check_output(
-            command, stderr=subprocess.STDOUT, shell=shell, env=environment, cwd=working_directory
-        )
-        return output.decode() if output is not None else None
-
     process = subprocess.Popen(
         command,
         stdin=input_file,
@@ -218,6 +211,33 @@ def execute_command(
     )
 
 
+def execute_command_and_capture_output(
+    full_command, capture_stderr=False, shell=False, extra_environment=None, working_directory=None,
+):
+    '''
+    Execute the given command (a sequence of command/argument strings), capturing and returning its
+    output (stdout). If capture stderr is True, then capture and return stderr in addition to
+    stdout. If shell is True, execute the command within a shell. If an extra environment dict is
+    given, then use it to augment the current environment, and pass the result into the command. If
+    a working directory is given, use that as the present working directory when running the command.
+
+    Raise subprocesses.CalledProcessError if an error occurs while running the command.
+    '''
+    log_command(full_command)
+    environment = {**os.environ, **extra_environment} if extra_environment else None
+    command = ' '.join(full_command) if shell else full_command
+
+    output = subprocess.check_output(
+        command,
+        stderr=subprocess.STDOUT if capture_stderr else None,
+        shell=shell,
+        env=environment,
+        cwd=working_directory,
+    )
+
+    return output.decode() if output is not None else None
+
+
 def execute_command_with_processes(
     full_command,
     processes,

+ 7 - 3
borgmatic/hooks/mysql.py

@@ -1,6 +1,10 @@
 import logging
 
-from borgmatic.execute import execute_command, execute_command_with_processes
+from borgmatic.execute import (
+    execute_command,
+    execute_command_and_capture_output,
+    execute_command_with_processes,
+)
 from borgmatic.hooks import dump
 
 logger = logging.getLogger(__name__)
@@ -42,8 +46,8 @@ def database_names_to_dump(database, extra_environment, log_prefix, dry_run_labe
     logger.debug(
         '{}: Querying for "all" MySQL databases to dump{}'.format(log_prefix, dry_run_label)
     )
-    show_output = execute_command(
-        show_command, output_log_level=None, extra_environment=extra_environment
+    show_output = execute_command_and_capture_output(
+        show_command, extra_environment=extra_environment
     )
 
     return tuple(

+ 1 - 1
setup.py

@@ -1,6 +1,6 @@
 from setuptools import find_packages, setup
 
-VERSION = '1.7.4.dev0'
+VERSION = '1.7.4'
 
 
 setup(

+ 11 - 19
tests/unit/borg/test_create.py

@@ -357,7 +357,7 @@ def test_any_parent_directories_treats_unrelated_paths_as_non_match():
 
 
 def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_list():
-    flexmock(module).should_receive('execute_command').and_return(
+    flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         'Processing files ...\n- /foo\n- /bar\n- /baz'
     )
     flexmock(module).should_receive('special_file').and_return(True)
@@ -373,7 +373,9 @@ def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_
 
 
 def test_collect_special_file_paths_excludes_requested_directories():
-    flexmock(module).should_receive('execute_command').and_return('- /foo\n- /bar\n- /baz')
+    flexmock(module).should_receive('execute_command_and_capture_output').and_return(
+        '- /foo\n- /bar\n- /baz'
+    )
     flexmock(module).should_receive('special_file').and_return(True)
     flexmock(module).should_receive('any_parent_directories').and_return(False).and_return(
         True
@@ -389,7 +391,9 @@ def test_collect_special_file_paths_excludes_requested_directories():
 
 
 def test_collect_special_file_paths_excludes_non_special_files():
-    flexmock(module).should_receive('execute_command').and_return('- /foo\n- /bar\n- /baz')
+    flexmock(module).should_receive('execute_command_and_capture_output').and_return(
+        '- /foo\n- /bar\n- /baz'
+    )
     flexmock(module).should_receive('special_file').and_return(True).and_return(False).and_return(
         True
     )
@@ -628,11 +632,8 @@ def test_create_archive_with_log_info_and_json_suppresses_most_borg_output():
         (f'repo::{DEFAULT_ARCHIVE_NAME}',)
     )
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
         ('borg', 'create') + REPO_ARCHIVE_WITH_PATHS + ('--json',),
-        output_log_level=None,
-        output_file=None,
-        borg_local_path='borg',
         working_directory=None,
         extra_environment=None,
     )
@@ -709,11 +710,8 @@ def test_create_archive_with_log_debug_and_json_suppresses_most_borg_output():
         (f'repo::{DEFAULT_ARCHIVE_NAME}',)
     )
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
         ('borg', 'create') + REPO_ARCHIVE_WITH_PATHS + ('--json',),
-        output_log_level=None,
-        output_file=None,
-        borg_local_path='borg',
         working_directory=None,
         extra_environment=None,
     )
@@ -2003,11 +2001,8 @@ def test_create_archive_with_json_calls_borg_with_json_parameter():
         (f'repo::{DEFAULT_ARCHIVE_NAME}',)
     )
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
         ('borg', 'create') + REPO_ARCHIVE_WITH_PATHS + ('--json',),
-        output_log_level=None,
-        output_file=None,
-        borg_local_path='borg',
         working_directory=None,
         extra_environment=None,
     ).and_return('[]')
@@ -2045,11 +2040,8 @@ def test_create_archive_with_stats_and_json_calls_borg_without_stats_parameter()
         (f'repo::{DEFAULT_ARCHIVE_NAME}',)
     )
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
         ('borg', 'create') + REPO_ARCHIVE_WITH_PATHS + ('--json',),
-        output_log_level=None,
-        output_file=None,
-        borg_local_path='borg',
         working_directory=None,
         extra_environment=None,
     ).and_return('[]')

+ 0 - 7
tests/unit/borg/test_extract.py

@@ -15,13 +15,6 @@ def insert_execute_command_mock(command, working_directory=None):
     ).once()
 
 
-def insert_execute_command_output_mock(command, result):
-    flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        command, output_log_level=None, borg_local_path=command[0], extra_environment=None,
-    ).and_return(result).once()
-
-
 def test_extract_last_archive_dry_run_calls_borg_with_last_archive():
     flexmock(module.rlist).should_receive('resolve_archive_name').and_return('archive')
     insert_execute_command_mock(('borg', 'extract', '--dry-run', 'repo::archive'))

+ 6 - 15
tests/unit/borg/test_info.py

@@ -53,11 +53,8 @@ def test_display_archives_info_with_log_info_and_json_suppresses_most_borg_outpu
     flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(('--json',))
     flexmock(module.flags).should_receive('make_repository_flags').and_return(('--repo', 'repo'))
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'info', '--json', '--repo', 'repo'),
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'info', '--json', '--repo', 'repo'), extra_environment=None,
     ).and_return('[]')
 
     insert_logging_mock(logging.INFO)
@@ -97,11 +94,8 @@ def test_display_archives_info_with_log_debug_and_json_suppresses_most_borg_outp
     flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(('--json',))
     flexmock(module.flags).should_receive('make_repository_flags').and_return(('--repo', 'repo'))
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'info', '--json', '--repo', 'repo'),
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'info', '--json', '--repo', 'repo'), extra_environment=None,
     ).and_return('[]')
 
     insert_logging_mock(logging.DEBUG)
@@ -120,11 +114,8 @@ def test_display_archives_info_with_json_calls_borg_with_json_parameter():
     flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(('--json',))
     flexmock(module.flags).should_receive('make_repository_flags').and_return(('--repo', 'repo'))
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'info', '--json', '--repo', 'repo'),
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'info', '--json', '--repo', 'repo'), extra_environment=None,
     ).and_return('[]')
 
     json_output = module.display_archives_info(

+ 4 - 10
tests/unit/borg/test_list.py

@@ -361,11 +361,8 @@ def test_list_archive_calls_borg_multiple_times_with_find_paths():
 
     flexmock(module.feature).should_receive('available').and_return(False)
     flexmock(module.rlist).should_receive('make_rlist_command').and_return(('borg', 'list', 'repo'))
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'list', 'repo'),
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'list', 'repo'), extra_environment=None,
     ).and_return('archive1\narchive2').once()
     flexmock(module).should_receive('make_list_command').and_return(
         ('borg', 'list', 'repo::archive1')
@@ -559,11 +556,8 @@ def test_list_archive_with_find_paths_allows_archive_filter_flag_but_only_passes
         remote_path=None,
     ).and_return(('borg', 'rlist', '--repo', 'repo'))
 
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'rlist', '--repo', 'repo'),
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'rlist', '--repo', 'repo'), extra_environment=None,
     ).and_return('archive1\narchive2').once()
 
     flexmock(module).should_receive('make_list_command').with_args(

+ 6 - 15
tests/unit/borg/test_rinfo.py

@@ -68,11 +68,8 @@ def test_display_repository_info_with_log_info_and_json_suppresses_most_borg_out
     flexmock(module.feature).should_receive('available').and_return(True)
     flexmock(module.flags).should_receive('make_repository_flags').and_return(('--repo', 'repo',))
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'rinfo', '--json', '--repo', 'repo'),
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'rinfo', '--json', '--repo', 'repo'), extra_environment=None,
     ).and_return('[]')
 
     insert_logging_mock(logging.INFO)
@@ -110,11 +107,8 @@ def test_display_repository_info_with_log_debug_and_json_suppresses_most_borg_ou
     flexmock(module.feature).should_receive('available').and_return(True)
     flexmock(module.flags).should_receive('make_repository_flags').and_return(('--repo', 'repo',))
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'rinfo', '--json', '--repo', 'repo'),
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'rinfo', '--json', '--repo', 'repo'), extra_environment=None,
     ).and_return('[]')
 
     insert_logging_mock(logging.DEBUG)
@@ -132,11 +126,8 @@ def test_display_repository_info_with_json_calls_borg_with_json_parameter():
     flexmock(module.feature).should_receive('available').and_return(True)
     flexmock(module.flags).should_receive('make_repository_flags').and_return(('--repo', 'repo',))
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'rinfo', '--json', '--repo', 'repo'),
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'rinfo', '--json', '--repo', 'repo'), extra_environment=None,
     ).and_return('[]')
 
     json_output = module.display_repository_info(

+ 13 - 32
tests/unit/borg/test_rlist.py

@@ -28,11 +28,8 @@ def test_resolve_archive_name_passes_through_non_latest_archive_name():
 def test_resolve_archive_name_calls_borg_with_parameters():
     expected_archive = 'archive-name'
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'list') + BORG_LIST_LATEST_ARGUMENTS,
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'list') + BORG_LIST_LATEST_ARGUMENTS, extra_environment=None,
     ).and_return(expected_archive + '\n')
 
     assert (
@@ -44,11 +41,8 @@ def test_resolve_archive_name_calls_borg_with_parameters():
 def test_resolve_archive_name_with_log_info_calls_borg_without_info_parameter():
     expected_archive = 'archive-name'
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'list') + BORG_LIST_LATEST_ARGUMENTS,
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'list') + BORG_LIST_LATEST_ARGUMENTS, extra_environment=None,
     ).and_return(expected_archive + '\n')
     insert_logging_mock(logging.INFO)
 
@@ -61,11 +55,8 @@ def test_resolve_archive_name_with_log_info_calls_borg_without_info_parameter():
 def test_resolve_archive_name_with_log_debug_calls_borg_without_debug_parameter():
     expected_archive = 'archive-name'
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'list') + BORG_LIST_LATEST_ARGUMENTS,
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'list') + BORG_LIST_LATEST_ARGUMENTS, extra_environment=None,
     ).and_return(expected_archive + '\n')
     insert_logging_mock(logging.DEBUG)
 
@@ -78,11 +69,8 @@ def test_resolve_archive_name_with_log_debug_calls_borg_without_debug_parameter(
 def test_resolve_archive_name_with_local_path_calls_borg_via_local_path():
     expected_archive = 'archive-name'
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg1', 'list') + BORG_LIST_LATEST_ARGUMENTS,
-        output_log_level=None,
-        borg_local_path='borg1',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg1', 'list') + BORG_LIST_LATEST_ARGUMENTS, extra_environment=None,
     ).and_return(expected_archive + '\n')
 
     assert (
@@ -96,10 +84,8 @@ def test_resolve_archive_name_with_local_path_calls_borg_via_local_path():
 def test_resolve_archive_name_with_remote_path_calls_borg_with_remote_path_parameters():
     expected_archive = 'archive-name'
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
         ('borg', 'list', '--remote-path', 'borg1') + BORG_LIST_LATEST_ARGUMENTS,
-        output_log_level=None,
-        borg_local_path='borg',
         extra_environment=None,
     ).and_return(expected_archive + '\n')
 
@@ -113,11 +99,8 @@ def test_resolve_archive_name_with_remote_path_calls_borg_with_remote_path_param
 
 def test_resolve_archive_name_without_archives_raises():
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        ('borg', 'list') + BORG_LIST_LATEST_ARGUMENTS,
-        output_log_level=None,
-        borg_local_path='borg',
-        extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        ('borg', 'list') + BORG_LIST_LATEST_ARGUMENTS, extra_environment=None,
     ).and_return('')
 
     with pytest.raises(ValueError):
@@ -128,10 +111,8 @@ def test_resolve_archive_name_with_lock_wait_calls_borg_with_lock_wait_parameter
     expected_archive = 'archive-name'
 
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
         ('borg', 'list', '--lock-wait', 'okay') + BORG_LIST_LATEST_ARGUMENTS,
-        output_log_level=None,
-        borg_local_path='borg',
         extra_environment=None,
     ).and_return(expected_archive + '\n')
 
@@ -385,7 +366,7 @@ def test_list_repository_with_json_returns_borg_output():
         remote_path=None,
     ).and_return(('borg', 'rlist', 'repo'))
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').and_return(json_output)
+    flexmock(module).should_receive('execute_command_and_capture_output').and_return(json_output)
 
     assert (
         module.list_repository(

+ 10 - 8
tests/unit/borg/test_version.py

@@ -10,22 +10,24 @@ from ..test_verbosity import insert_logging_mock
 VERSION = '1.2.3'
 
 
-def insert_execute_command_mock(command, borg_local_path='borg', version_output=f'borg {VERSION}'):
+def insert_execute_command_and_capture_output_mock(
+    command, borg_local_path='borg', version_output=f'borg {VERSION}'
+):
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('execute_command').with_args(
-        command, output_log_level=None, borg_local_path=borg_local_path, extra_environment=None,
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
+        command, extra_environment=None,
     ).once().and_return(version_output)
 
 
 def test_local_borg_version_calls_borg_with_required_parameters():
-    insert_execute_command_mock(('borg', '--version'))
+    insert_execute_command_and_capture_output_mock(('borg', '--version'))
     flexmock(module.environment).should_receive('make_environment')
 
     assert module.local_borg_version({}) == VERSION
 
 
 def test_local_borg_version_with_log_info_calls_borg_with_info_parameter():
-    insert_execute_command_mock(('borg', '--version', '--info'))
+    insert_execute_command_and_capture_output_mock(('borg', '--version', '--info'))
     insert_logging_mock(logging.INFO)
     flexmock(module.environment).should_receive('make_environment')
 
@@ -33,7 +35,7 @@ def test_local_borg_version_with_log_info_calls_borg_with_info_parameter():
 
 
 def test_local_borg_version_with_log_debug_calls_borg_with_debug_parameters():
-    insert_execute_command_mock(('borg', '--version', '--debug', '--show-rc'))
+    insert_execute_command_and_capture_output_mock(('borg', '--version', '--debug', '--show-rc'))
     insert_logging_mock(logging.DEBUG)
     flexmock(module.environment).should_receive('make_environment')
 
@@ -41,14 +43,14 @@ def test_local_borg_version_with_log_debug_calls_borg_with_debug_parameters():
 
 
 def test_local_borg_version_with_local_borg_path_calls_borg_with_it():
-    insert_execute_command_mock(('borg1', '--version'), borg_local_path='borg1')
+    insert_execute_command_and_capture_output_mock(('borg1', '--version'), borg_local_path='borg1')
     flexmock(module.environment).should_receive('make_environment')
 
     assert module.local_borg_version({}, 'borg1') == VERSION
 
 
 def test_local_borg_version_with_invalid_version_raises():
-    insert_execute_command_mock(('borg', '--version'), version_output='wtf')
+    insert_execute_command_and_capture_output_mock(('borg', '--version'), version_output='wtf')
     flexmock(module.environment).should_receive('make_environment')
 
     with pytest.raises(ValueError):

+ 2 - 4
tests/unit/hooks/test_mysql.py

@@ -22,9 +22,8 @@ def test_database_names_to_dump_queries_mysql_for_database_names():
     extra_environment = flexmock()
     log_prefix = ''
     dry_run_label = ''
-    flexmock(module).should_receive('execute_command').with_args(
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
         ('mysql', '--skip-column-names', '--batch', '--execute', 'show schemas'),
-        output_log_level=None,
         extra_environment=extra_environment,
     ).and_return('foo\nbar\nmysql\n').once()
 
@@ -200,7 +199,7 @@ def test_dump_databases_runs_mysqldump_for_all_databases():
 
 def test_database_names_to_dump_runs_mysql_with_list_options():
     database = {'name': 'all', 'list_options': '--defaults-extra-file=my.cnf'}
-    flexmock(module).should_receive('execute_command').with_args(
+    flexmock(module).should_receive('execute_command_and_capture_output').with_args(
         (
             'mysql',
             '--defaults-extra-file=my.cnf',
@@ -209,7 +208,6 @@ def test_database_names_to_dump_runs_mysql_with_list_options():
             '--execute',
             'show schemas',
         ),
-        output_log_level=None,
         extra_environment=None,
     ).and_return(('foo\nbar')).once()
 

+ 26 - 17
tests/unit/test_execute.py

@@ -213,7 +213,20 @@ def test_execute_command_without_run_to_completion_returns_process():
     assert module.execute_command(full_command, run_to_completion=False) == process
 
 
-def test_execute_command_captures_output():
+def test_execute_command_and_capture_output_returns_stdout():
+    full_command = ['foo', 'bar']
+    expected_output = '[]'
+    flexmock(module.os, environ={'a': 'b'})
+    flexmock(module.subprocess).should_receive('check_output').with_args(
+        full_command, stderr=None, shell=False, env=None, cwd=None
+    ).and_return(flexmock(decode=lambda: expected_output)).once()
+
+    output = module.execute_command_and_capture_output(full_command)
+
+    assert output == expected_output
+
+
+def test_execute_command_and_capture_output_with_capture_stderr_returns_stderr():
     full_command = ['foo', 'bar']
     expected_output = '[]'
     flexmock(module.os, environ={'a': 'b'})
@@ -221,53 +234,49 @@ def test_execute_command_captures_output():
         full_command, stderr=module.subprocess.STDOUT, shell=False, env=None, cwd=None
     ).and_return(flexmock(decode=lambda: expected_output)).once()
 
-    output = module.execute_command(full_command, output_log_level=None)
+    output = module.execute_command_and_capture_output(full_command, capture_stderr=True)
 
     assert output == expected_output
 
 
-def test_execute_command_captures_output_with_shell():
+def test_execute_command_and_capture_output_returns_output_with_shell():
     full_command = ['foo', 'bar']
     expected_output = '[]'
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('check_output').with_args(
-        'foo bar', stderr=module.subprocess.STDOUT, shell=True, env=None, cwd=None
+        'foo bar', stderr=None, shell=True, env=None, cwd=None
     ).and_return(flexmock(decode=lambda: expected_output)).once()
 
-    output = module.execute_command(full_command, output_log_level=None, shell=True)
+    output = module.execute_command_and_capture_output(full_command, shell=True)
 
     assert output == expected_output
 
 
-def test_execute_command_captures_output_with_extra_environment():
+def test_execute_command_and_capture_output_returns_output_with_extra_environment():
     full_command = ['foo', 'bar']
     expected_output = '[]'
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('check_output').with_args(
-        full_command,
-        stderr=module.subprocess.STDOUT,
-        shell=False,
-        env={'a': 'b', 'c': 'd'},
-        cwd=None,
+        full_command, stderr=None, shell=False, env={'a': 'b', 'c': 'd'}, cwd=None,
     ).and_return(flexmock(decode=lambda: expected_output)).once()
 
-    output = module.execute_command(
-        full_command, output_log_level=None, shell=False, extra_environment={'c': 'd'}
+    output = module.execute_command_and_capture_output(
+        full_command, shell=False, extra_environment={'c': 'd'}
     )
 
     assert output == expected_output
 
 
-def test_execute_command_captures_output_with_working_directory():
+def test_execute_command_and_capture_output_returns_output_with_working_directory():
     full_command = ['foo', 'bar']
     expected_output = '[]'
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('check_output').with_args(
-        full_command, stderr=module.subprocess.STDOUT, shell=False, env=None, cwd='/working'
+        full_command, stderr=None, shell=False, env=None, cwd='/working'
     ).and_return(flexmock(decode=lambda: expected_output)).once()
 
-    output = module.execute_command(
-        full_command, output_log_level=None, shell=False, working_directory='/working'
+    output = module.execute_command_and_capture_output(
+        full_command, shell=False, working_directory='/working'
     )
 
     assert output == expected_output