From 1c88dda76ad95a72b3e8ab5f92a9f84eab10dc13 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 2 Apr 2019 22:30:14 -0700 Subject: [PATCH] Fix for invalid JSON output when using multiple borgmatic configuration files (#155). --- NEWS | 6 +- borgmatic/borg/create.py | 8 +- borgmatic/borg/execute.py | 19 ++++ borgmatic/borg/info.py | 12 ++- borgmatic/borg/list.py | 12 +-- borgmatic/commands/borgmatic.py | 92 +++++++++---------- setup.py | 2 +- tests/unit/borg/test_create.py | 125 ++++++++++++++++++-------- tests/unit/borg/test_execute.py | 25 ++++++ tests/unit/borg/test_info.py | 35 +++++--- tests/unit/borg/test_list.py | 39 +++++--- tests/unit/commands/test_borgmatic.py | 65 ++++---------- 12 files changed, 257 insertions(+), 183 deletions(-) create mode 100644 borgmatic/borg/execute.py create mode 100644 tests/unit/borg/test_execute.py diff --git a/NEWS b/NEWS index 0b91a703..09a6315e 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,10 @@ -1.3.1.dev0 +1.3.1 + * #155: Fix for invalid JSON output when using multiple borgmatic configuration files. * #157: Fix for seemingly random filename ordering when running through a directory of configuration files. + * Fix for empty JSON output when using --create --json. + * Now capturing Borg output only when --json flag is used. Previously, borgmatic delayed Borg + output even without the --json flag. 1.3.0 * #148: Configuration file includes and merging via "!include" tag to support reuse of common diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 62ecb8be..d420cdec 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -2,9 +2,10 @@ import glob import itertools import logging import os -import subprocess import tempfile +from borgmatic.borg.execute import execute_command + logger = logging.getLogger(__name__) @@ -107,7 +108,7 @@ def create_archive( ): ''' Given vebosity/dry-run flags, a local or remote repository path, a location config dict, and a - storage config dict, create a Borg archive. + storage config dict, create a Borg archive and return Borg's JSON output (if any). ''' sources = _expand_directories(location_config['source_directories']) @@ -157,5 +158,4 @@ def create_archive( + (('--json',) if json else ()) ) - logger.debug(' '.join(full_command)) - subprocess.check_call(full_command) + return execute_command(full_command, capture_output=json) diff --git a/borgmatic/borg/execute.py b/borgmatic/borg/execute.py new file mode 100644 index 00000000..064557af --- /dev/null +++ b/borgmatic/borg/execute.py @@ -0,0 +1,19 @@ +import logging +import subprocess + + +logger = logging.getLogger(__name__) + + +def execute_command(full_command, capture_output=False): + ''' + Execute the given command (a sequence of command/argument strings). If capture output is True, + then return the command's output as a string. + ''' + logger.debug(' '.join(full_command)) + + if capture_output: + output = subprocess.check_output(full_command) + return output.decode() if output is not None else None + else: + subprocess.check_call(full_command) diff --git a/borgmatic/borg/info.py b/borgmatic/borg/info.py index 2fddd90d..79bfb5ac 100644 --- a/borgmatic/borg/info.py +++ b/borgmatic/borg/info.py @@ -1,5 +1,6 @@ import logging -import subprocess + +from borgmatic.borg.execute import execute_command logger = logging.getLogger(__name__) @@ -9,8 +10,8 @@ def display_archives_info( repository, storage_config, local_path='borg', remote_path=None, json=False ): ''' - Given a local or remote repository path, and a storage config dict, - display summary information for Borg archives in the repository. + Given a local or remote repository path, and a storage config dict, display summary information + for Borg archives in the repository or return JSON summary information. ''' lock_wait = storage_config.get('lock_wait', None) @@ -23,7 +24,4 @@ def display_archives_info( + (('--json',) if json else ()) ) - logger.debug(' '.join(full_command)) - - output = subprocess.check_output(full_command) - return output.decode() if output is not None else None + return execute_command(full_command, capture_output=json) diff --git a/borgmatic/borg/list.py b/borgmatic/borg/list.py index 3c2f6f9d..0c2b8bc5 100644 --- a/borgmatic/borg/list.py +++ b/borgmatic/borg/list.py @@ -1,5 +1,6 @@ import logging -import subprocess + +from borgmatic.borg.execute import execute_command logger = logging.getLogger(__name__) @@ -9,8 +10,9 @@ def list_archives( repository, storage_config, archive=None, local_path='borg', remote_path=None, json=False ): ''' - Given a local or remote repository path and a storage config dict, list Borg archives in the - repository. Or, if an archive name is given, list the files in that archive. + Given a local or remote repository path and a storage config dict, display the output of listing + Borg archives in the repository or return JSON output. Or, if an archive name is given, listing + the files in that archive. ''' lock_wait = storage_config.get('lock_wait', None) @@ -22,7 +24,5 @@ def list_archives( + (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ()) + (('--json',) if json else ()) ) - logger.debug(' '.join(full_command)) - output = subprocess.check_output(full_command) - return output.decode() if output is not None else None + return execute_command(full_command, capture_output=json) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 41ce4cec..4ab5a75b 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -258,6 +258,8 @@ def run_configuration(config_filename, config, args): # pragma: no cover ''' Given a config filename and the corresponding parsed config dict, execute its defined pruning, backups, consistency checks, and/or other actions. + + Yield JSON output strings from executing any actions that produce JSON. ''' (location, storage, retention, consistency, hooks) = ( config.get(section_name, {}) @@ -272,15 +274,17 @@ def run_configuration(config_filename, config, args): # pragma: no cover if args.create: hook.execute_hook(hooks.get('before_backup'), config_filename, 'pre-backup') - _run_commands( - args=args, - consistency=consistency, - local_path=local_path, - location=location, - remote_path=remote_path, - retention=retention, - storage=storage, - ) + for repository_path in location['repositories']: + yield from run_actions( + args=args, + location=location, + storage=storage, + retention=retention, + consistency=consistency, + local_path=local_path, + remote_path=remote_path, + repository_path=repository_path, + ) if args.create: hook.execute_hook(hooks.get('after_backup'), config_filename, 'post-backup') @@ -289,37 +293,17 @@ def run_configuration(config_filename, config, args): # pragma: no cover raise -def _run_commands(*, args, consistency, local_path, location, remote_path, retention, storage): - json_results = [] - for unexpanded_repository in location['repositories']: - _run_commands_on_repository( - args=args, - consistency=consistency, - json_results=json_results, - local_path=local_path, - location=location, - remote_path=remote_path, - retention=retention, - storage=storage, - unexpanded_repository=unexpanded_repository, - ) - if args.json: - sys.stdout.write(json.dumps(json_results)) - - -def _run_commands_on_repository( - *, - args, - consistency, - json_results, - local_path, - location, - remote_path, - retention, - storage, - unexpanded_repository +def run_actions( + *, args, location, storage, retention, consistency, local_path, remote_path, repository_path ): # pragma: no cover - repository = os.path.expanduser(unexpanded_repository) + ''' + Given parsed command-line arguments as an argparse.ArgumentParser instance, several different + configuration dicts, local and remote paths to Borg, and a repository name, run all actions + from the command-line arguments on the given repository. + + Yield JSON output strings from executing any actions that produce JSON. + ''' + repository = os.path.expanduser(repository_path) dry_run_label = ' (dry run; not making any changes)' if args.dry_run else '' if args.init: logger.info('{}: Initializing repository'.format(repository)) @@ -344,7 +328,7 @@ def _run_commands_on_repository( ) if args.create: logger.info('{}: Creating archive{}'.format(repository, dry_run_label)) - borg_create.create_archive( + json_output = borg_create.create_archive( args.dry_run, repository, location, @@ -353,7 +337,10 @@ def _run_commands_on_repository( remote_path=remote_path, progress=args.progress, stats=args.stats, + json=args.json, ) + if json_output: + yield json.loads(json_output) if args.check and checks.repository_enabled_for_checks(repository, consistency): logger.info('{}: Running consistency checks'.format(repository)) borg_check.check_archives( @@ -376,7 +363,7 @@ def _run_commands_on_repository( if args.list: if args.repository is None or repository == args.repository: logger.info('{}: Listing archives'.format(repository)) - output = borg_list.list_archives( + json_output = borg_list.list_archives( repository, storage, args.archive, @@ -384,19 +371,15 @@ def _run_commands_on_repository( remote_path=remote_path, json=args.json, ) - if args.json: - json_results.append(json.loads(output)) - else: - sys.stdout.write(output) + if json_output: + yield json.loads(json_output) if args.info: logger.info('{}: Displaying summary info for archives'.format(repository)) - output = borg_info.display_archives_info( + json_output = borg_info.display_archives_info( repository, storage, local_path=local_path, remote_path=remote_path, json=args.json ) - if args.json: - json_results.append(json.loads(output)) - else: - sys.stdout.write(output) + if json_output: + yield json.loads(json_output) def collect_configuration_run_summary_logs(config_filenames, args): @@ -404,6 +387,9 @@ def collect_configuration_run_summary_logs(config_filenames, args): Given a sequence of configuration filenames and parsed command-line arguments as an argparse.ArgumentParser instance, run each configuration file and yield a series of logging.LogRecord instances containing summary information about each run. + + As a side effect of running through these configuration files, output their JSON results, if + any, to stdout. ''' # Dict mapping from config filename to corresponding parsed config dict. configs = collections.OrderedDict() @@ -433,9 +419,10 @@ def collect_configuration_run_summary_logs(config_filenames, args): return # Execute the actions corresponding to each configuration file. + json_results = [] for config_filename, config in configs.items(): try: - run_configuration(config_filename, config, args) + json_results.extend(list(run_configuration(config_filename, config, args))) yield logging.makeLogRecord( dict( levelno=logging.INFO, @@ -451,6 +438,9 @@ def collect_configuration_run_summary_logs(config_filenames, args): ) yield logging.makeLogRecord(dict(levelno=logging.CRITICAL, msg=error)) + if json_results: + sys.stdout.write(json.dumps(json_results)) + if not config_filenames: yield logging.makeLogRecord( dict( diff --git a/setup.py b/setup.py index 6682e995..134e91f7 100644 --- a/setup.py +++ b/setup.py @@ -1,7 +1,7 @@ from setuptools import setup, find_packages -VERSION = '1.3.1.dev0' +VERSION = '1.3.1' setup( diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index e2ac7f11..18914fa4 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -69,11 +69,6 @@ def test_write_pattern_file_with_empty_exclude_patterns_does_not_raise(): module._write_pattern_file([]) -def insert_subprocess_mock(check_call_command, **kwargs): - subprocess = flexmock(module.subprocess) - subprocess.should_receive('check_call').with_args(check_call_command, **kwargs).once() - - def test_make_pattern_flags_includes_pattern_filename_when_given(): pattern_flags = module._make_pattern_flags( location_config={'patterns': ['R /', '- /var']}, pattern_filename='/tmp/patterns' @@ -169,7 +164,9 @@ def test_create_archive_calls_borg_with_parameters(): 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(()) - insert_subprocess_mock(CREATE_COMMAND) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND, capture_output=False + ) module.create_archive( dry_run=False, @@ -192,7 +189,9 @@ def test_create_archive_with_patterns_calls_borg_with_patterns(): ).and_return(None) flexmock(module).should_receive('_make_pattern_flags').and_return(pattern_flags) flexmock(module).should_receive('_make_exclude_flags').and_return(()) - insert_subprocess_mock(CREATE_COMMAND + pattern_flags) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + pattern_flags, capture_output=False + ) module.create_archive( dry_run=False, @@ -215,7 +214,9 @@ def test_create_archive_with_exclude_patterns_calls_borg_with_excludes(): ) flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(exclude_flags) - insert_subprocess_mock(CREATE_COMMAND + exclude_flags) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + exclude_flags, capture_output=False + ) module.create_archive( dry_run=False, @@ -236,7 +237,9 @@ def test_create_archive_with_log_info_calls_borg_with_info_parameter(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) - insert_subprocess_mock(CREATE_COMMAND + ('--list', '--filter', 'AME-', '--info', '--stats')) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--list', '--filter', 'AME-', '--info', '--stats'), capture_output=False + ) insert_logging_mock(logging.INFO) module.create_archive( @@ -257,8 +260,9 @@ def test_create_archive_with_log_debug_calls_borg_with_debug_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(()) - insert_subprocess_mock( - CREATE_COMMAND + ('--list', '--filter', 'AME-', '--stats', '--debug', '--show-rc') + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--list', '--filter', 'AME-', '--stats', '--debug', '--show-rc'), + capture_output=False, ) insert_logging_mock(logging.DEBUG) @@ -281,7 +285,9 @@ def test_create_archive_with_dry_run_calls_borg_with_dry_run_parameter(): flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) - insert_subprocess_mock(CREATE_COMMAND + ('--dry-run',)) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--dry-run',), capture_output=False + ) module.create_archive( dry_run=True, @@ -304,7 +310,9 @@ def test_create_archive_with_dry_run_and_log_info_calls_borg_without_stats_param flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) - insert_subprocess_mock(CREATE_COMMAND + ('--list', '--filter', 'AME-', '--info', '--dry-run')) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--list', '--filter', 'AME-', '--info', '--dry-run'), capture_output=False + ) insert_logging_mock(logging.INFO) module.create_archive( @@ -328,8 +336,9 @@ def test_create_archive_with_dry_run_and_log_debug_calls_borg_without_stats_para flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_pattern_flags').and_return(()) flexmock(module).should_receive('_make_exclude_flags').and_return(()) - insert_subprocess_mock( - CREATE_COMMAND + ('--list', '--filter', 'AME-', '--debug', '--show-rc', '--dry-run') + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--list', '--filter', 'AME-', '--debug', '--show-rc', '--dry-run'), + capture_output=False, ) insert_logging_mock(logging.DEBUG) @@ -351,7 +360,9 @@ def test_create_archive_with_checkpoint_interval_calls_borg_with_checkpoint_inte 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--checkpoint-interval', '600')) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--checkpoint-interval', '600'), capture_output=False + ) module.create_archive( dry_run=False, @@ -371,7 +382,9 @@ def test_create_archive_with_chunker_params_calls_borg_with_chunker_params_param 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--chunker-params', '1,2,3,4')) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--chunker-params', '1,2,3,4'), capture_output=False + ) module.create_archive( dry_run=False, @@ -391,7 +404,9 @@ def test_create_archive_with_compression_calls_borg_with_compression_parameters( 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--compression', 'rle')) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--compression', 'rle'), capture_output=False + ) module.create_archive( dry_run=False, @@ -411,7 +426,9 @@ def test_create_archive_with_remote_rate_limit_calls_borg_with_remote_ratelimit_ 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--remote-ratelimit', '100')) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--remote-ratelimit', '100'), capture_output=False + ) module.create_archive( dry_run=False, @@ -431,7 +448,9 @@ def test_create_archive_with_one_file_system_calls_borg_with_one_file_system_par 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--one-file-system',)) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--one-file-system',), capture_output=False + ) module.create_archive( dry_run=False, @@ -452,7 +471,9 @@ def test_create_archive_with_numeric_owner_calls_borg_with_numeric_owner_paramet 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--numeric-owner',)) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--numeric-owner',), capture_output=False + ) module.create_archive( dry_run=False, @@ -473,7 +494,9 @@ def test_create_archive_with_read_special_calls_borg_with_read_special_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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--read-special',)) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--read-special',), capture_output=False + ) module.create_archive( dry_run=False, @@ -494,7 +517,9 @@ def test_create_archive_with_bsd_flags_true_calls_borg_without_nobsdflags_parame 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(()) - insert_subprocess_mock(CREATE_COMMAND) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND, capture_output=False + ) module.create_archive( dry_run=False, @@ -515,7 +540,9 @@ def test_create_archive_with_bsd_flags_false_calls_borg_with_nobsdflags_paramete 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--nobsdflags',)) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--nobsdflags',), capture_output=False + ) module.create_archive( dry_run=False, @@ -536,7 +563,9 @@ def test_create_archive_with_files_cache_calls_borg_with_files_cache_parameters( 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--files-cache', 'ctime,size')) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--files-cache', 'ctime,size'), capture_output=False + ) module.create_archive( dry_run=False, @@ -557,7 +586,9 @@ def test_create_archive_with_local_path_calls_borg_via_local_path(): 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(()) - insert_subprocess_mock(('borg1',) + CREATE_COMMAND[1:]) + flexmock(module).should_receive('execute_command').with_args( + ('borg1',) + CREATE_COMMAND[1:], capture_output=False + ) module.create_archive( dry_run=False, @@ -578,7 +609,9 @@ def test_create_archive_with_remote_path_calls_borg_with_remote_path_parameters( 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--remote-path', 'borg1')) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--remote-path', 'borg1'), capture_output=False + ) module.create_archive( dry_run=False, @@ -599,7 +632,9 @@ def test_create_archive_with_umask_calls_borg_with_umask_parameters(): 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--umask', '740')) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--umask', '740'), capture_output=False + ) module.create_archive( dry_run=False, @@ -619,7 +654,9 @@ def test_create_archive_with_lock_wait_calls_borg_with_lock_wait_parameters(): 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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--lock-wait', '5')) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--lock-wait', '5'), capture_output=False + ) module.create_archive( dry_run=False, @@ -639,9 +676,11 @@ def test_create_archive_with_json_calls_borg_with_json_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(()) - insert_subprocess_mock(CREATE_COMMAND + ('--json',)) + flexmock(module).should_receive('execute_command').with_args( + CREATE_COMMAND + ('--json',), capture_output=True + ).and_return('[]') - module.create_archive( + json_output = module.create_archive( dry_run=False, repository='repo', location_config={ @@ -653,6 +692,8 @@ def test_create_archive_with_json_calls_borg_with_json_parameter(): json=True, ) + assert json_output == '[]' + def test_create_archive_with_source_directories_glob_expands(): flexmock(module).should_receive('_expand_directories').and_return(('foo', 'food')) @@ -660,8 +701,9 @@ def test_create_archive_with_source_directories_glob_expands(): 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(()) - insert_subprocess_mock( - ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food') + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food'), + capture_output=False, ) flexmock(module.glob).should_receive('glob').with_args('foo*').and_return(['foo', 'food']) @@ -683,7 +725,9 @@ def test_create_archive_with_non_matching_source_directories_glob_passes_through 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(()) - insert_subprocess_mock(('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo*')) + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo*'), capture_output=False + ) flexmock(module.glob).should_receive('glob').with_args('foo*').and_return([]) module.create_archive( @@ -704,8 +748,9 @@ def test_create_archive_with_glob_calls_borg_with_expanded_directories(): 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(()) - insert_subprocess_mock( - ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food') + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food'), + capture_output=False, ) module.create_archive( @@ -726,7 +771,9 @@ def test_create_archive_with_archive_name_format_calls_borg_with_archive_name(): 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(()) - insert_subprocess_mock(('borg', 'create', 'repo::ARCHIVE_NAME', 'foo', 'bar')) + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'create', 'repo::ARCHIVE_NAME', 'foo', 'bar'), capture_output=False + ) module.create_archive( dry_run=False, @@ -746,7 +793,9 @@ def test_create_archive_with_archive_name_format_accepts_borg_placeholders(): 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(()) - insert_subprocess_mock(('borg', 'create', 'repo::Documents_{hostname}-{now}', 'foo', 'bar')) + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'create', 'repo::Documents_{hostname}-{now}', 'foo', 'bar'), capture_output=False + ) module.create_archive( dry_run=False, diff --git a/tests/unit/borg/test_execute.py b/tests/unit/borg/test_execute.py new file mode 100644 index 00000000..6bb2938c --- /dev/null +++ b/tests/unit/borg/test_execute.py @@ -0,0 +1,25 @@ +from flexmock import flexmock +from borgmatic.borg import execute as module + + +def test_execute_command_calls_full_command(): + full_command = ['foo', 'bar'] + subprocess = flexmock(module.subprocess) + subprocess.should_receive('check_call').with_args(full_command).once() + + output = module.execute_command(full_command) + + assert output is None + + +def test_execute_command_captures_output(): + full_command = ['foo', 'bar'] + expected_output = '[]' + subprocess = flexmock(module.subprocess) + subprocess.should_receive('check_output').with_args(full_command).and_return( + flexmock(decode=lambda: expected_output) + ).once() + + output = module.execute_command(full_command, capture_output=True) + + assert output == expected_output diff --git a/tests/unit/borg/test_info.py b/tests/unit/borg/test_info.py index 7e79456d..a3abc61b 100644 --- a/tests/unit/borg/test_info.py +++ b/tests/unit/borg/test_info.py @@ -6,53 +6,62 @@ from borgmatic.borg import info as module from ..test_verbosity import insert_logging_mock -def insert_subprocess_mock(check_call_command, **kwargs): - subprocess = flexmock(module.subprocess) - subprocess.should_receive('check_output').with_args(check_call_command, **kwargs).once() - - INFO_COMMAND = ('borg', 'info', 'repo') def test_display_archives_info_calls_borg_with_parameters(): - insert_subprocess_mock(INFO_COMMAND) + flexmock(module).should_receive('execute_command').with_args(INFO_COMMAND, capture_output=False) module.display_archives_info(repository='repo', storage_config={}) def test_display_archives_info_with_log_info_calls_borg_with_info_parameter(): - insert_subprocess_mock(INFO_COMMAND + ('--info',)) + flexmock(module).should_receive('execute_command').with_args( + INFO_COMMAND + ('--info',), capture_output=False + ) insert_logging_mock(logging.INFO) module.display_archives_info(repository='repo', storage_config={}) def test_display_archives_info_with_log_debug_calls_borg_with_debug_parameter(): - insert_subprocess_mock(INFO_COMMAND + ('--debug', '--show-rc')) + flexmock(module).should_receive('execute_command').with_args( + INFO_COMMAND + ('--debug', '--show-rc'), capture_output=False + ) insert_logging_mock(logging.DEBUG) module.display_archives_info(repository='repo', storage_config={}) def test_display_archives_info_with_json_calls_borg_with_json_parameter(): - insert_subprocess_mock(INFO_COMMAND + ('--json',)) + flexmock(module).should_receive('execute_command').with_args( + INFO_COMMAND + ('--json',), capture_output=True + ).and_return('[]') - module.display_archives_info(repository='repo', storage_config={}, json=True) + json_output = module.display_archives_info(repository='repo', storage_config={}, json=True) + + assert json_output == '[]' def test_display_archives_info_with_local_path_calls_borg_via_local_path(): - insert_subprocess_mock(('borg1',) + INFO_COMMAND[1:]) + flexmock(module).should_receive('execute_command').with_args( + ('borg1',) + INFO_COMMAND[1:], capture_output=False + ) module.display_archives_info(repository='repo', storage_config={}, local_path='borg1') def test_display_archives_info_with_remote_path_calls_borg_with_remote_path_parameters(): - insert_subprocess_mock(INFO_COMMAND + ('--remote-path', 'borg1')) + flexmock(module).should_receive('execute_command').with_args( + INFO_COMMAND + ('--remote-path', 'borg1'), capture_output=False + ) module.display_archives_info(repository='repo', storage_config={}, remote_path='borg1') def test_display_archives_info_with_lock_wait_calls_borg_with_lock_wait_parameters(): storage_config = {'lock_wait': 5} - insert_subprocess_mock(INFO_COMMAND + ('--lock-wait', '5')) + flexmock(module).should_receive('execute_command').with_args( + INFO_COMMAND + ('--lock-wait', '5'), capture_output=False + ) module.display_archives_info(repository='repo', storage_config=storage_config) diff --git a/tests/unit/borg/test_list.py b/tests/unit/borg/test_list.py index ed19c4e6..ea354b32 100644 --- a/tests/unit/borg/test_list.py +++ b/tests/unit/borg/test_list.py @@ -6,29 +6,28 @@ from borgmatic.borg import list as module from ..test_verbosity import insert_logging_mock -def insert_subprocess_mock(check_call_command, **kwargs): - subprocess = flexmock(module.subprocess) - subprocess.should_receive('check_output').with_args(check_call_command, **kwargs).once() - - LIST_COMMAND = ('borg', 'list', 'repo') def test_list_archives_calls_borg_with_parameters(): - insert_subprocess_mock(LIST_COMMAND) + flexmock(module).should_receive('execute_command').with_args(LIST_COMMAND, capture_output=False) module.list_archives(repository='repo', storage_config={}) def test_list_archives_with_log_info_calls_borg_with_info_parameter(): - insert_subprocess_mock(LIST_COMMAND + ('--info',)) + flexmock(module).should_receive('execute_command').with_args( + LIST_COMMAND + ('--info',), capture_output=False + ) insert_logging_mock(logging.INFO) module.list_archives(repository='repo', storage_config={}) def test_list_archives_with_log_debug_calls_borg_with_debug_parameter(): - insert_subprocess_mock(LIST_COMMAND + ('--debug', '--show-rc')) + flexmock(module).should_receive('execute_command').with_args( + LIST_COMMAND + ('--debug', '--show-rc'), capture_output=False + ) insert_logging_mock(logging.DEBUG) module.list_archives(repository='repo', storage_config={}) @@ -36,31 +35,43 @@ def test_list_archives_with_log_debug_calls_borg_with_debug_parameter(): def test_list_archives_with_lock_wait_calls_borg_with_lock_wait_parameters(): storage_config = {'lock_wait': 5} - insert_subprocess_mock(LIST_COMMAND + ('--lock-wait', '5')) + flexmock(module).should_receive('execute_command').with_args( + LIST_COMMAND + ('--lock-wait', '5'), capture_output=False + ) module.list_archives(repository='repo', storage_config=storage_config) def test_list_archives_with_archive_calls_borg_with_archive_parameter(): storage_config = {} - insert_subprocess_mock(('borg', 'list', 'repo::archive')) + flexmock(module).should_receive('execute_command').with_args( + ('borg', 'list', 'repo::archive'), capture_output=False + ) module.list_archives(repository='repo', storage_config=storage_config, archive='archive') def test_list_archives_with_local_path_calls_borg_via_local_path(): - insert_subprocess_mock(('borg1',) + LIST_COMMAND[1:]) + flexmock(module).should_receive('execute_command').with_args( + ('borg1',) + LIST_COMMAND[1:], capture_output=False + ) module.list_archives(repository='repo', storage_config={}, local_path='borg1') def test_list_archives_with_remote_path_calls_borg_with_remote_path_parameters(): - insert_subprocess_mock(LIST_COMMAND + ('--remote-path', 'borg1')) + flexmock(module).should_receive('execute_command').with_args( + LIST_COMMAND + ('--remote-path', 'borg1'), capture_output=False + ) module.list_archives(repository='repo', storage_config={}, remote_path='borg1') def test_list_archives_with_json_calls_borg_with_json_parameter(): - insert_subprocess_mock(LIST_COMMAND + ('--json',)) + flexmock(module).should_receive('execute_command').with_args( + LIST_COMMAND + ('--json',), capture_output=True + ).and_return('[]') - module.list_archives(repository='repo', storage_config={}, json=True) + json_output = module.list_archives(repository='repo', storage_config={}, json=True) + + assert json_output == '[]' diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index d87294a3..e5610878 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -1,55 +1,11 @@ -import json -import sys - from flexmock import flexmock from borgmatic.commands import borgmatic as module -def test_run_commands_handles_multiple_json_outputs_in_array(): - ( - flexmock(module) - .should_receive('_run_commands_on_repository') - .times(3) - .replace_with( - lambda args, consistency, json_results, local_path, location, remote_path, retention, storage, unexpanded_repository: json_results.append( - {"whatever": unexpanded_repository} - ) - ) - ) - - ( - flexmock(sys.stdout) - .should_call("write") - .with_args( - json.dumps( - json.loads( - ''' - [ - {"whatever": "fake_repo1"}, - {"whatever": "fake_repo2"}, - {"whatever": "fake_repo3"} - ] - ''' - ) - ) - ) - ) - - module._run_commands( - args=flexmock(json=True), - consistency=None, - local_path=None, - location={'repositories': ['fake_repo1', 'fake_repo2', 'fake_repo3']}, - remote_path=None, - retention=None, - storage=None, - ) - - def test_collect_configuration_run_summary_logs_info_for_success(): flexmock(module.validate).should_receive('parse_configuration').and_return({'test.yaml': {}}) - flexmock(module).should_receive('run_configuration') + flexmock(module).should_receive('run_configuration').and_return([]) args = flexmock(extract=False, list=False) logs = tuple(module.collect_configuration_run_summary_logs(('test.yaml',), args=args)) @@ -60,7 +16,7 @@ def test_collect_configuration_run_summary_logs_info_for_success(): def test_collect_configuration_run_summary_logs_info_for_success_with_extract(): flexmock(module.validate).should_receive('parse_configuration').and_return({'test.yaml': {}}) flexmock(module.validate).should_receive('guard_configuration_contains_repository') - flexmock(module).should_receive('run_configuration') + flexmock(module).should_receive('run_configuration').and_return([]) args = flexmock(extract=True, list=False, repository='repo') logs = tuple(module.collect_configuration_run_summary_logs(('test.yaml',), args=args)) @@ -94,7 +50,7 @@ def test_collect_configuration_run_summary_logs_critical_for_list_with_archive_a def test_collect_configuration_run_summary_logs_info_for_success_with_list(): flexmock(module.validate).should_receive('parse_configuration').and_return({'test.yaml': {}}) - flexmock(module).should_receive('run_configuration') + flexmock(module).should_receive('run_configuration').and_return([]) args = flexmock(extract=False, list=True, repository='repo', archive=None) logs = tuple(module.collect_configuration_run_summary_logs(('test.yaml',), args=args)) @@ -124,9 +80,22 @@ def test_collect_configuration_run_summary_logs_critical_for_run_error(): def test_collect_configuration_run_summary_logs_critical_for_missing_configs(): flexmock(module.validate).should_receive('parse_configuration').and_return({'test.yaml': {}}) - flexmock(module).should_receive('run_configuration') + flexmock(module).should_receive('run_configuration').and_return([]) args = flexmock(config_paths=(), extract=False, list=False) logs = tuple(module.collect_configuration_run_summary_logs(config_filenames=(), args=args)) assert any(log for log in logs if log.levelno == module.logging.CRITICAL) + + +def test_collect_configuration_run_summary_logs_outputs_merged_json_results(): + flexmock(module.validate).should_receive('parse_configuration').and_return( + {'test.yaml': {}, 'test2.yaml': {}} + ) + flexmock(module).should_receive('run_configuration').and_return(['foo', 'bar']).and_return( + ['baz'] + ) + flexmock(module.sys.stdout).should_receive('write').with_args('["foo", "bar", "baz"]').once() + args = flexmock(extract=False, list=False) + + tuple(module.collect_configuration_run_summary_logs(('test.yaml', 'test2.yaml'), args=args))