From 9c42e7e8173a731a99008a2676db526af88f2170 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 14 Oct 2022 16:19:26 -0700 Subject: [PATCH] Fix regression in which "check" action errored on certain systems (#597, #598). --- NEWS | 6 ++-- borgmatic/borg/create.py | 34 ++++++++++++--------- borgmatic/borg/info.py | 19 +++++++----- borgmatic/borg/list.py | 6 ++-- borgmatic/borg/rinfo.py | 21 ++++++++----- borgmatic/borg/rlist.py | 25 +++++++--------- borgmatic/borg/version.py | 9 ++---- borgmatic/execute.py | 52 +++++++++++++++++++++++---------- borgmatic/hooks/mysql.py | 10 +++++-- setup.py | 2 +- tests/unit/borg/test_create.py | 30 +++++++------------ tests/unit/borg/test_extract.py | 7 ----- tests/unit/borg/test_info.py | 21 ++++--------- tests/unit/borg/test_list.py | 14 +++------ tests/unit/borg/test_rinfo.py | 21 ++++--------- tests/unit/borg/test_rlist.py | 45 +++++++++------------------- tests/unit/borg/test_version.py | 18 +++++++----- tests/unit/hooks/test_mysql.py | 6 ++-- tests/unit/test_execute.py | 43 ++++++++++++++++----------- 19 files changed, 189 insertions(+), 200 deletions(-) diff --git a/NEWS b/NEWS index 38e3b15..51763c6 100644 --- a/NEWS +++ b/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 diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index efe092f..ef42578 100644 --- a/borgmatic/borg/create.py +++ b/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, + ) diff --git a/borgmatic/borg/info.py b/borgmatic/borg/info.py index cea3607..a665173 100644 --- a/borgmatic/borg/info.py +++ b/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), + ) diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index c426ab3..3d493b1 100644 --- a/borgmatic/borg/list.py +++ b/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') diff --git a/borgmatic/borg/rinfo.py b/borgmatic/borg/rinfo.py index 3fc8bb5..317472d 100644 --- a/borgmatic/borg/rinfo.py +++ b/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, + ) diff --git a/borgmatic/borg/rlist.py b/borgmatic/borg/rlist.py index e64e353..e823e1b 100644 --- a/borgmatic/borg/rlist.py +++ b/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, + ) diff --git a/borgmatic/borg/version.py b/borgmatic/borg/version.py index 61766e4..6d6c302 100644 --- a/borgmatic/borg/version.py +++ b/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: diff --git a/borgmatic/execute.py b/borgmatic/execute.py index 00a5fc9..c3ddfdf 100644 --- a/borgmatic/execute.py +++ b/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, diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index 3cc8255..c2d91e6 100644 --- a/borgmatic/hooks/mysql.py +++ b/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( diff --git a/setup.py b/setup.py index 4f49624..27ff223 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.7.4.dev0' +VERSION = '1.7.4' setup( diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 4dd390b..a999fe3 100644 --- a/tests/unit/borg/test_create.py +++ b/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('[]') diff --git a/tests/unit/borg/test_extract.py b/tests/unit/borg/test_extract.py index d572ff9..64d16d3 100644 --- a/tests/unit/borg/test_extract.py +++ b/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')) diff --git a/tests/unit/borg/test_info.py b/tests/unit/borg/test_info.py index 140bed6..afe16e5 100644 --- a/tests/unit/borg/test_info.py +++ b/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( diff --git a/tests/unit/borg/test_list.py b/tests/unit/borg/test_list.py index 8ce32fc..92d6e94 100644 --- a/tests/unit/borg/test_list.py +++ b/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( diff --git a/tests/unit/borg/test_rinfo.py b/tests/unit/borg/test_rinfo.py index b3147b9..fe15a4d 100644 --- a/tests/unit/borg/test_rinfo.py +++ b/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( diff --git a/tests/unit/borg/test_rlist.py b/tests/unit/borg/test_rlist.py index d6ece7b..42ba495 100644 --- a/tests/unit/borg/test_rlist.py +++ b/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( diff --git a/tests/unit/borg/test_version.py b/tests/unit/borg/test_version.py index 6931e26..66789a8 100644 --- a/tests/unit/borg/test_version.py +++ b/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): diff --git a/tests/unit/hooks/test_mysql.py b/tests/unit/hooks/test_mysql.py index 631e500..35516c3 100644 --- a/tests/unit/hooks/test_mysql.py +++ b/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() diff --git a/tests/unit/test_execute.py b/tests/unit/test_execute.py index 9bab8db..0441e9d 100644 --- a/tests/unit/test_execute.py +++ b/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