From 1ea047dd94995c7550f9a63f555df2877bb3d415 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 9 May 2020 21:53:16 -0700 Subject: [PATCH] Remove "borgmatic restore --progress" flag, as it now conflicts with streaming database restores. --- NEWS | 1 + borgmatic/borg/check.py | 4 +- borgmatic/borg/create.py | 20 ++--- borgmatic/borg/extract.py | 12 ++- borgmatic/borg/init.py | 6 +- borgmatic/borg/mount.py | 4 +- borgmatic/commands/arguments.py | 7 -- borgmatic/commands/borgmatic.py | 1 - borgmatic/execute.py | 84 +++++++++++--------- tests/integration/commands/test_arguments.py | 6 -- tests/unit/borg/test_check.py | 12 ++- tests/unit/borg/test_create.py | 50 +++++++++++- tests/unit/borg/test_extract.py | 3 +- tests/unit/borg/test_init.py | 9 +-- tests/unit/borg/test_mount.py | 6 +- tests/unit/test_execute.py | 27 ------- 16 files changed, 133 insertions(+), 119 deletions(-) diff --git a/NEWS b/NEWS index bddcae80f..220689229 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ filesystem space. This feature is automatic, and works even on restores from archives made with previous versions of borgmatic. * #293: Documentation on macOS launchd permissions issues with work-around for Full Disk Access. + * Remove "borgmatic restore --progress" flag, as it now conflicts with streaming database restores. 1.5.2 * #301: Fix MySQL restore error on "all" database dump by excluding system tables. diff --git a/borgmatic/borg/check.py b/borgmatic/borg/check.py index 55332157b..d2bd5c9a9 100644 --- a/borgmatic/borg/check.py +++ b/borgmatic/borg/check.py @@ -1,7 +1,7 @@ import logging from borgmatic.borg import extract -from borgmatic.execute import execute_command, execute_command_without_capture +from borgmatic.execute import DO_NOT_CAPTURE, execute_command DEFAULT_CHECKS = ('repository', 'archives') DEFAULT_PREFIX = '{hostname}-' @@ -134,7 +134,7 @@ def check_archives( # The Borg repair option trigger an interactive prompt, which won't work when output is # captured. And progress messes with the terminal directly. if repair or progress: - execute_command_without_capture(full_command, error_on_warnings=True) + execute_command(full_command, output_file=DO_NOT_CAPTURE, error_on_warnings=True) else: execute_command(full_command, error_on_warnings=True) diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 12c0b3b7d..5954f50e2 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -4,11 +4,7 @@ import logging import os import tempfile -from borgmatic.execute import ( - execute_command, - execute_command_with_processes, - execute_command_without_capture, -) +from borgmatic.execute import DO_NOT_CAPTURE, execute_command, execute_command_with_processes logger = logging.getLogger(__name__) @@ -206,12 +202,6 @@ def create_archive( + sources ) - # The progress output isn't compatible with captured and logged output, as progress messes with - # the terminal directly. - if progress: - execute_command_without_capture(full_command, error_on_warnings=False) - return - if json: output_log_level = None elif (stats or files) and logger.getEffectiveLevel() == logging.WARNING: @@ -219,9 +209,13 @@ def create_archive( else: output_log_level = logging.INFO + # The progress output isn't compatible with captured and logged output, as progress messes with + # the terminal directly. + output_file = DO_NOT_CAPTURE if progress else None + if stream_processes: return execute_command_with_processes( - full_command, stream_processes, output_log_level, error_on_warnings=False + full_command, stream_processes, output_log_level, output_file, error_on_warnings=False ) - return execute_command(full_command, output_log_level, error_on_warnings=False) + return execute_command(full_command, output_log_level, output_file, error_on_warnings=False) diff --git a/borgmatic/borg/extract.py b/borgmatic/borg/extract.py index 419069b5d..78584d776 100644 --- a/borgmatic/borg/extract.py +++ b/borgmatic/borg/extract.py @@ -2,7 +2,7 @@ import logging import os import subprocess -from borgmatic.execute import execute_command, execute_command_without_capture +from borgmatic.execute import DO_NOT_CAPTURE, execute_command logger = logging.getLogger(__name__) @@ -78,6 +78,9 @@ def extract_archive( umask = storage_config.get('umask', None) lock_wait = storage_config.get('lock_wait', None) + if progress and extract_to_stdout: + raise ValueError('progress and extract_to_stdout cannot both be set') + full_command = ( (local_path, 'extract') + (('--remote-path', remote_path) if remote_path else ()) @@ -96,8 +99,11 @@ def extract_archive( # The progress output isn't compatible with captured and logged output, as progress messes with # the terminal directly. if progress: - execute_command_without_capture( - full_command, working_directory=destination_path, error_on_warnings=error_on_warnings + return execute_command( + full_command, + output_file=DO_NOT_CAPTURE, + working_directory=destination_path, + error_on_warnings=error_on_warnings, ) return None diff --git a/borgmatic/borg/init.py b/borgmatic/borg/init.py index 08256aefa..24aa8c4d3 100644 --- a/borgmatic/borg/init.py +++ b/borgmatic/borg/init.py @@ -1,7 +1,7 @@ import logging import subprocess -from borgmatic.execute import execute_command, execute_command_without_capture +from borgmatic.execute import DO_NOT_CAPTURE, execute_command logger = logging.getLogger(__name__) @@ -54,5 +54,5 @@ def initialize_repository( + (repository,) ) - # Don't use execute_command() here because it doesn't support interactive prompts. - execute_command_without_capture(init_command, error_on_warnings=False) + # Do not capture output here, so as to support interactive prompts. + execute_command(init_command, output_file=DO_NOT_CAPTURE, error_on_warnings=False) diff --git a/borgmatic/borg/mount.py b/borgmatic/borg/mount.py index 4fccbf9ff..bf3fc9295 100644 --- a/borgmatic/borg/mount.py +++ b/borgmatic/borg/mount.py @@ -1,6 +1,6 @@ import logging -from borgmatic.execute import execute_command, execute_command_without_capture +from borgmatic.execute import DO_NOT_CAPTURE, execute_command logger = logging.getLogger(__name__) @@ -40,7 +40,7 @@ def mount_archive( # Don't capture the output when foreground mode is used so that ctrl-C can work properly. if foreground: - execute_command_without_capture(full_command, error_on_warnings=False) + execute_command(full_command, output_file=DO_NOT_CAPTURE, error_on_warnings=False) return execute_command(full_command, error_on_warnings=False) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index e366909b7..2bf6e8e9a 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -427,13 +427,6 @@ def parse_arguments(*unparsed_arguments): dest='databases', help='Names of databases to restore from archive, defaults to all databases. Note that any databases to restore must be defined in borgmatic\'s configuration', ) - restore_group.add_argument( - '--progress', - dest='progress', - default=False, - action='store_true', - help='Display progress for each database dump file as it is extracted from archive', - ) restore_group.add_argument( '-h', '--help', action='help', help='Show this help message and exit' ) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index bafda3b8e..14dd0d3c8 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -386,7 +386,6 @@ def run_actions( local_path=local_path, remote_path=remote_path, destination_path='/', - progress=arguments['restore'].progress, extract_to_stdout=True, ) diff --git a/borgmatic/execute.py b/borgmatic/execute.py index d4e9d2e40..96450d69c 100644 --- a/borgmatic/execute.py +++ b/borgmatic/execute.py @@ -89,7 +89,11 @@ def log_many_outputs(processes, exclude_stdouts, output_log_level, error_on_warn ''' # Map from output buffer to sequence of last lines. buffer_last_lines = collections.defaultdict(list) - output_buffers = [output_buffer_for_process(process, exclude_stdouts) for process in processes] + output_buffers = [ + output_buffer_for_process(process, exclude_stdouts) + for process in processes + if process.stdout or process.stderr + ] while True: (ready_buffers, _, _) = select.select(output_buffers, [], []) @@ -112,9 +116,13 @@ def log_many_outputs(processes, exclude_stdouts, output_log_level, error_on_warn break for process in processes: - remaining_output = ( - output_buffer_for_process(process, exclude_stdouts).read().rstrip().decode() - ) + output_buffer = output_buffer_for_process(process, exclude_stdouts) + + if not output_buffer: + continue + + remaining_output = output_buffer.read().rstrip().decode() + if remaining_output: # pragma: no cover logger.log(output_log_level, remaining_output) @@ -125,7 +133,8 @@ def log_many_outputs(processes, exclude_stdouts, output_log_level, error_on_warn # If an error occurs, include its output in the raised exception so that we don't # inadvertently hide error output. output_buffer = output_buffer_for_process(process, exclude_stdouts) - last_lines = buffer_last_lines[output_buffer] + + last_lines = buffer_last_lines[output_buffer] if output_buffer else [] if len(last_lines) == ERROR_OUTPUT_MAX_LINE_COUNT: last_lines.insert(0, '...') @@ -146,6 +155,9 @@ def log_command(full_command, input_file, output_file): ) +DO_NOT_CAPTURE = object() + + def execute_command( full_command, output_log_level=logging.INFO, @@ -173,49 +185,40 @@ def execute_command( ''' log_command(full_command, input_file, output_file) environment = {**os.environ, **extra_environment} if extra_environment else None + do_not_capture = bool(output_file is DO_NOT_CAPTURE) if output_log_level is None: output = subprocess.check_output( full_command, shell=shell, env=environment, cwd=working_directory ) return output.decode() if output is not None else None - else: - process = subprocess.Popen( - ' '.join(full_command) if shell else full_command, - stdin=input_file, - stdout=output_file or subprocess.PIPE, - stderr=subprocess.PIPE if output_file else subprocess.STDOUT, - shell=shell, - env=environment, - cwd=working_directory, - ) - if not run_to_completion: - return process - log_output( - process, - process.stderr if output_file else process.stdout, - output_log_level, - error_on_warnings, - ) + process = subprocess.Popen( + ' '.join(full_command) if shell else full_command, + stdin=input_file, + stdout=None if do_not_capture else (output_file or subprocess.PIPE), + stderr=None if do_not_capture else (subprocess.PIPE if output_file else subprocess.STDOUT), + shell=shell, + env=environment, + cwd=working_directory, + ) + if not run_to_completion: + return process + if do_not_capture: + exit_code = process.poll() -def execute_command_without_capture(full_command, working_directory=None, error_on_warnings=True): - ''' - Execute the given command (a sequence of command/argument strings), but don't capture or log its - output in any way. This is necessary for commands that monkey with the terminal (e.g. progress - display) or provide interactive prompts. + if exit_code_indicates_error(exit_code, error_on_warnings): + raise subprocess.CalledProcessError(exit_code, process_command(process)) - If a working directory is given, use that as the present working directory when running the - command. If error on warnings is False, then treat exit code 1 as a warning instead of an error. - ''' - logger.debug(' '.join(full_command)) + return - try: - subprocess.check_call(full_command, cwd=working_directory) - except subprocess.CalledProcessError as error: - if exit_code_indicates_error(error.returncode, error_on_warnings): - raise + log_output( + process, + process.stderr if output_file else process.stdout, + output_log_level, + error_on_warnings, + ) def execute_command_with_processes( @@ -248,13 +251,16 @@ def execute_command_with_processes( ''' log_command(full_command, input_file, output_file) environment = {**os.environ, **extra_environment} if extra_environment else None + do_not_capture = bool(output_file is DO_NOT_CAPTURE) try: command_process = subprocess.Popen( full_command, stdin=input_file, - stdout=output_file or subprocess.PIPE, - stderr=subprocess.PIPE if output_file else subprocess.STDOUT, + stdout=None if do_not_capture else (output_file or subprocess.PIPE), + stderr=None + if do_not_capture + else (subprocess.PIPE if output_file else subprocess.STDOUT), shell=shell, env=environment, cwd=working_directory, diff --git a/tests/integration/commands/test_arguments.py b/tests/integration/commands/test_arguments.py index 46bc380c0..7871d4aa2 100644 --- a/tests/integration/commands/test_arguments.py +++ b/tests/integration/commands/test_arguments.py @@ -393,12 +393,6 @@ def test_parse_arguments_allows_progress_and_extract(): module.parse_arguments('--progress', 'extract', '--archive', 'test', 'list') -def test_parse_arguments_allows_progress_and_restore(): - flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) - - module.parse_arguments('--progress', 'restore', '--archive', 'test', 'list') - - def test_parse_arguments_disallows_progress_without_create(): flexmock(module.collect).should_receive('get_default_config_paths').and_return(['default']) diff --git a/tests/unit/borg/test_check.py b/tests/unit/borg/test_check.py index 96b9979ef..6de6a8f98 100644 --- a/tests/unit/borg/test_check.py +++ b/tests/unit/borg/test_check.py @@ -164,8 +164,10 @@ def test_check_archives_with_progress_calls_borg_with_progress_parameter(): flexmock(module).should_receive('_parse_checks').and_return(checks) flexmock(module).should_receive('_make_check_flags').and_return(()) flexmock(module).should_receive('execute_command').never() - flexmock(module).should_receive('execute_command_without_capture').with_args( - ('borg', 'check', '--progress', 'repo'), error_on_warnings=True + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'check', '--progress', 'repo'), + output_file=module.DO_NOT_CAPTURE, + error_on_warnings=True, ).once() module.check_archives( @@ -179,8 +181,10 @@ def test_check_archives_with_repair_calls_borg_with_repair_parameter(): flexmock(module).should_receive('_parse_checks').and_return(checks) flexmock(module).should_receive('_make_check_flags').and_return(()) flexmock(module).should_receive('execute_command').never() - flexmock(module).should_receive('execute_command_without_capture').with_args( - ('borg', 'check', '--repair', 'repo'), error_on_warnings=True + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'check', '--repair', 'repo'), + output_file=module.DO_NOT_CAPTURE, + error_on_warnings=True, ).once() module.check_archives( diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 428df2d25..bf09e5084 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -222,6 +222,7 @@ def test_create_archive_calls_borg_with_parameters(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -250,6 +251,7 @@ def test_create_archive_with_patterns_calls_borg_with_patterns(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create') + pattern_flags + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -278,6 +280,7 @@ def test_create_archive_with_exclude_patterns_calls_borg_with_excludes(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create') + exclude_flags + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -304,6 +307,7 @@ def test_create_archive_with_log_info_calls_borg_with_info_parameter(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--info') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -331,6 +335,7 @@ def test_create_archive_with_log_info_and_json_suppresses_most_borg_output(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, output_log_level=None, + output_file=None, error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -358,6 +363,7 @@ def test_create_archive_with_log_debug_calls_borg_with_debug_parameter(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--debug', '--show-rc') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) insert_logging_mock(logging.DEBUG) @@ -384,6 +390,7 @@ def test_create_archive_with_log_debug_and_json_suppresses_most_borg_output(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, output_log_level=None, + output_file=None, error_on_warnings=False, ) insert_logging_mock(logging.DEBUG) @@ -412,6 +419,7 @@ def test_create_archive_with_dry_run_calls_borg_with_dry_run_parameter(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--dry-run') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -440,6 +448,7 @@ def test_create_archive_with_stats_and_dry_run_calls_borg_without_stats_paramete flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--info', '--dry-run') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -467,6 +476,7 @@ def test_create_archive_with_checkpoint_interval_calls_borg_with_checkpoint_inte flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--checkpoint-interval', '600') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -492,6 +502,7 @@ def test_create_archive_with_chunker_params_calls_borg_with_chunker_params_param flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--chunker-params', '1,2,3,4') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -517,6 +528,7 @@ def test_create_archive_with_compression_calls_borg_with_compression_parameters( flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--compression', 'rle') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -542,6 +554,7 @@ def test_create_archive_with_remote_rate_limit_calls_borg_with_remote_ratelimit_ flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--remote-ratelimit', '100') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -567,6 +580,7 @@ def test_create_archive_with_one_file_system_calls_borg_with_one_file_system_par flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--one-file-system') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -593,6 +607,7 @@ def test_create_archive_with_numeric_owner_calls_borg_with_numeric_owner_paramet flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--numeric-owner') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -619,6 +634,7 @@ def test_create_archive_with_read_special_calls_borg_with_read_special_parameter flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--read-special') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -646,6 +662,7 @@ def test_create_archive_with_option_true_calls_borg_without_corresponding_parame flexmock(module).should_receive('execute_command').with_args( ('borg', 'create') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -673,6 +690,7 @@ def test_create_archive_with_option_false_calls_borg_with_corresponding_paramete flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--no' + option_name.replace('_', '')) + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -699,6 +717,7 @@ def test_create_archive_with_files_cache_calls_borg_with_files_cache_parameters( flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--files-cache', 'ctime,size') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -725,6 +744,7 @@ def test_create_archive_with_local_path_calls_borg_via_local_path(): flexmock(module).should_receive('execute_command').with_args( ('borg1', 'create') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -751,6 +771,7 @@ def test_create_archive_with_remote_path_calls_borg_with_remote_path_parameters( flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--remote-path', 'borg1') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -777,6 +798,7 @@ def test_create_archive_with_umask_calls_borg_with_umask_parameters(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--umask', '740') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -802,6 +824,7 @@ def test_create_archive_with_lock_wait_calls_borg_with_lock_wait_parameters(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--lock-wait', '5') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -827,6 +850,7 @@ def test_create_archive_with_stats_calls_borg_with_stats_parameter_and_warning_o flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--stats') + ARCHIVE_WITH_PATHS, output_log_level=logging.WARNING, + output_file=None, error_on_warnings=False, ) @@ -853,6 +877,7 @@ def test_create_archive_with_stats_and_log_info_calls_borg_with_stats_parameter_ flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--info', '--stats') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -880,6 +905,7 @@ def test_create_archive_with_files_calls_borg_with_list_parameter_and_warning_ou flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--list', '--filter', 'AME-') + ARCHIVE_WITH_PATHS, output_log_level=logging.WARNING, + output_file=None, error_on_warnings=False, ) @@ -906,6 +932,7 @@ def test_create_archive_with_files_and_log_info_calls_borg_with_list_parameter_a flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--list', '--filter', 'AME-', '--info') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -930,8 +957,11 @@ def test_create_archive_with_progress_and_log_info_calls_borg_with_progress_para flexmock(module).should_receive('_write_pattern_file').and_return(None) flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) - flexmock(module).should_receive('execute_command_without_capture').with_args( - ('borg', 'create', '--info', '--progress') + ARCHIVE_WITH_PATHS, error_on_warnings=False + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'create', '--info', '--progress') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + output_file=module.DO_NOT_CAPTURE, + error_on_warnings=False, ) insert_logging_mock(logging.INFO) @@ -955,8 +985,11 @@ def test_create_archive_with_progress_calls_borg_with_progress_parameter(): flexmock(module).should_receive('_write_pattern_file').and_return(None) flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) - flexmock(module).should_receive('execute_command_without_capture').with_args( - ('borg', 'create', '--progress') + ARCHIVE_WITH_PATHS, error_on_warnings=False + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'create', '--progress') + ARCHIVE_WITH_PATHS, + output_log_level=logging.INFO, + output_file=module.DO_NOT_CAPTURE, + error_on_warnings=False, ) module.create_archive( @@ -982,6 +1015,7 @@ def test_create_archive_with_json_calls_borg_with_json_parameter(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, output_log_level=None, + output_file=None, error_on_warnings=False, ).and_return('[]') @@ -1010,6 +1044,7 @@ def test_create_archive_with_stats_and_json_calls_borg_without_stats_parameter() flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--json') + ARCHIVE_WITH_PATHS, output_log_level=None, + output_file=None, error_on_warnings=False, ).and_return('[]') @@ -1039,6 +1074,7 @@ def test_create_archive_with_source_directories_glob_expands(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food'), output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) flexmock(module.glob).should_receive('glob').with_args('foo*').and_return(['foo', 'food']) @@ -1065,6 +1101,7 @@ def test_create_archive_with_non_matching_source_directories_glob_passes_through flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo*'), output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) flexmock(module.glob).should_receive('glob').with_args('foo*').and_return([]) @@ -1091,6 +1128,7 @@ def test_create_archive_with_glob_calls_borg_with_expanded_directories(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food'), output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -1116,6 +1154,7 @@ def test_create_archive_with_archive_name_format_calls_borg_with_archive_name(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', 'repo::ARCHIVE_NAME', 'foo', 'bar'), output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -1141,6 +1180,7 @@ def test_create_archive_with_archive_name_format_accepts_borg_placeholders(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', 'repo::Documents_{hostname}-{now}', 'foo', 'bar'), output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -1166,6 +1206,7 @@ def test_create_archive_with_extra_borg_options_calls_borg_with_extra_options(): flexmock(module).should_receive('execute_command').with_args( ('borg', 'create', '--extra', '--options') + ARCHIVE_WITH_PATHS, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) @@ -1193,6 +1234,7 @@ def test_create_archive_with_stream_processes_calls_borg_with_processes(): ('borg', 'create', '--read-special') + ARCHIVE_WITH_PATHS, processes=processes, output_log_level=logging.INFO, + output_file=None, error_on_warnings=False, ) diff --git a/tests/unit/borg/test_extract.py b/tests/unit/borg/test_extract.py index 3723dc418..95ed2accb 100644 --- a/tests/unit/borg/test_extract.py +++ b/tests/unit/borg/test_extract.py @@ -221,8 +221,9 @@ def test_extract_archive_calls_borg_with_destination_path(): def test_extract_archive_calls_borg_with_progress_parameter(): flexmock(module.os.path).should_receive('abspath').and_return('repo') - flexmock(module).should_receive('execute_command_without_capture').with_args( + flexmock(module).should_receive('execute_command').with_args( ('borg', 'extract', '--progress', 'repo::archive'), + output_file=module.DO_NOT_CAPTURE, working_directory=None, error_on_warnings=True, ).once() diff --git a/tests/unit/borg/test_init.py b/tests/unit/borg/test_init.py index 49e9933c3..3745b33fb 100644 --- a/tests/unit/borg/test_init.py +++ b/tests/unit/borg/test_init.py @@ -23,8 +23,8 @@ def insert_info_command_not_found_mock(): def insert_init_command_mock(init_command, **kwargs): - flexmock(module).should_receive('execute_command_without_capture').with_args( - init_command, error_on_warnings=False + flexmock(module).should_receive('execute_command').with_args( + init_command, output_file=module.DO_NOT_CAPTURE, error_on_warnings=False ).once() @@ -37,7 +37,7 @@ def test_initialize_repository_calls_borg_with_parameters(): def test_initialize_repository_raises_for_borg_init_error(): insert_info_command_not_found_mock() - flexmock(module).should_receive('execute_command_without_capture').and_raise( + flexmock(module).should_receive('execute_command').and_raise( module.subprocess.CalledProcessError(2, 'borg init') ) @@ -48,8 +48,7 @@ def test_initialize_repository_raises_for_borg_init_error(): def test_initialize_repository_skips_initialization_when_repository_already_exists(): - insert_info_command_found_mock() - flexmock(module).should_receive('execute_command_without_capture').never() + flexmock(module).should_receive('execute_command').once() module.initialize_repository(repository='repo', storage_config={}, encryption_mode='repokey') diff --git a/tests/unit/borg/test_mount.py b/tests/unit/borg/test_mount.py index 109140632..d605d1adb 100644 --- a/tests/unit/borg/test_mount.py +++ b/tests/unit/borg/test_mount.py @@ -117,8 +117,10 @@ def test_mount_archive_with_log_debug_calls_borg_with_debug_parameters(): def test_mount_archive_calls_borg_with_foreground_parameter(): - flexmock(module).should_receive('execute_command_without_capture').with_args( - ('borg', 'mount', '--foreground', 'repo::archive', '/mnt'), error_on_warnings=False + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'mount', '--foreground', 'repo::archive', '/mnt'), + output_file=module.DO_NOT_CAPTURE, + error_on_warnings=False, ).once() module.mount_archive( diff --git a/tests/unit/test_execute.py b/tests/unit/test_execute.py index 9fd34654b..dd18a8071 100644 --- a/tests/unit/test_execute.py +++ b/tests/unit/test_execute.py @@ -225,30 +225,3 @@ def test_execute_command_captures_output_with_working_directory(): ) assert output == expected_output - - -def test_execute_command_without_capture_does_not_raise_on_success(): - flexmock(module.subprocess).should_receive('check_call').and_raise( - module.subprocess.CalledProcessError(0, 'borg init') - ) - - module.execute_command_without_capture(('borg', 'init')) - - -def test_execute_command_without_capture_does_not_raise_on_warning(): - flexmock(module).should_receive('exit_code_indicates_error').and_return(False) - flexmock(module.subprocess).should_receive('check_call').and_raise( - module.subprocess.CalledProcessError(1, 'borg init') - ) - - module.execute_command_without_capture(('borg', 'init')) - - -def test_execute_command_without_capture_raises_on_error(): - flexmock(module).should_receive('exit_code_indicates_error').and_return(True) - flexmock(module.subprocess).should_receive('check_call').and_raise( - module.subprocess.CalledProcessError(2, 'borg init') - ) - - with pytest.raises(module.subprocess.CalledProcessError): - module.execute_command_without_capture(('borg', 'init'))