From 8ad7b473f1c0a2c232b5ebc0f4ae358daba069d9 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 26 Oct 2023 22:12:13 -0700 Subject: [PATCH] When an archive filter causes no matching archives for the "rlist" or "info" actions, warn (#748). --- NEWS | 2 + borgmatic/borg/flags.py | 27 ++ borgmatic/borg/info.py | 84 ++++-- borgmatic/borg/rlist.py | 34 ++- tests/integration/borg/test_commands.py | 3 + tests/unit/borg/test_flags.py | 50 ++++ tests/unit/borg/test_info.py | 367 ++++++++++-------------- tests/unit/borg/test_rinfo.py | 51 ++++ tests/unit/borg/test_rlist.py | 55 +--- 9 files changed, 389 insertions(+), 284 deletions(-) diff --git a/NEWS b/NEWS index d7083025..6541dc69 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ * #715: Add a monitoring hook for sending backup status to a variety of monitoring services via the Apprise library. See the documentation for more information: https://torsion.org/borgmatic/docs/how-to/monitor-your-backups/#apprise-hook + * #748: When an archive filter causes no matching archives for the "rlist" or "info" + actions, warn the user and suggest how to remove the filter. * #768: Fix a traceback when an invalid command-line flag or action is used. * #771: Fix normalization of deprecated sections ("location:", "storage:", "hooks:", etc.) to support empty sections without erroring. diff --git a/borgmatic/borg/flags.py b/borgmatic/borg/flags.py index bcfadfa4..c06bcc9a 100644 --- a/borgmatic/borg/flags.py +++ b/borgmatic/borg/flags.py @@ -1,8 +1,12 @@ import itertools +import json +import logging import re from borgmatic.borg import feature +logger = logging.getLogger(__name__) + def make_flags(name, value): ''' @@ -86,3 +90,26 @@ def make_match_archives_flags(match_archives, archive_name_format, local_borg_ve return ('--match-archives', f'sh:{derived_match_archives}') else: return ('--glob-archives', f'{derived_match_archives}') + + +def warn_for_aggressive_archive_flags(json_command, json_output): + ''' + Given a JSON archives command and the resulting JSON string output from running it, parse the + JSON and warn if the command used an archive flag but the output indicates zero archives were + found. + ''' + archive_flags_used = {'--glob-archives', '--match-archives'}.intersection(set(json_command)) + + if not archive_flags_used: + return + + try: + if len(json.loads(json_output)['archives']) == 0: + logger.warning('An archive filter was applied, but no matching archives were found.') + logger.warning( + 'Try adding --match-archives "*" or adjusting archive_name_format/match_archives in configuration.' + ) + except json.JSONDecodeError as error: + logger.debug(f'Cannot parse JSON output from archive command: {error}') + except (TypeError, KeyError): + logger.debug('Cannot parse JSON output from archive command: No "archives" key found') diff --git a/borgmatic/borg/info.py b/borgmatic/borg/info.py index 3e596ca4..cf31a737 100644 --- a/borgmatic/borg/info.py +++ b/borgmatic/borg/info.py @@ -1,3 +1,4 @@ +import argparse import logging import borgmatic.logger @@ -7,24 +8,21 @@ from borgmatic.execute import execute_command, execute_command_and_capture_outpu logger = logging.getLogger(__name__) -def display_archives_info( +def make_info_command( repository_path, config, local_borg_version, info_arguments, global_arguments, - local_path='borg', - remote_path=None, + local_path, + remote_path, ): ''' - Given a local or remote repository path, a configuration dict, the local Borg version, global - arguments as an argparse.Namespace, and the arguments to the info action, display summary - information for Borg archives in the repository or return JSON summary information. + Given a local or remote repository path, a configuration dict, the local Borg version, the + arguments to the info action as an argparse.Namespace, and global arguments, return a command + as a tuple to display summary information for archives in the repository. ''' - borgmatic.logger.add_custom_log_levels() - lock_wait = config.get('lock_wait', None) - - full_command = ( + return ( (local_path, 'info') + ( ('--info',) @@ -38,7 +36,7 @@ def display_archives_info( ) + flags.make_flags('remote-path', remote_path) + flags.make_flags('log-json', global_arguments.log_json) - + flags.make_flags('lock-wait', lock_wait) + + flags.make_flags('lock-wait', config.get('lock_wait')) + ( ( flags.make_flags('match-archives', f'sh:{info_arguments.prefix}*') @@ -62,16 +60,56 @@ def display_archives_info( + flags.make_repository_flags(repository_path, local_borg_version) ) + +def display_archives_info( + repository_path, + config, + local_borg_version, + info_arguments, + global_arguments, + local_path='borg', + remote_path=None, +): + ''' + Given a local or remote repository path, a configuration dict, the local Borg version, the + arguments to the info action as an argparse.Namespace, and global arguments, display summary + information for Borg archives in the repository or return JSON summary information. + ''' + borgmatic.logger.add_custom_log_levels() + + main_command = make_info_command( + repository_path, + config, + local_borg_version, + info_arguments, + global_arguments, + local_path, + remote_path, + ) + json_command = make_info_command( + repository_path, + config, + local_borg_version, + argparse.Namespace(**dict(info_arguments.__dict__, json=True)), + global_arguments, + local_path, + remote_path, + ) + + json_info = execute_command_and_capture_output( + json_command, + extra_environment=environment.make_environment(config), + borg_local_path=local_path, + ) + if info_arguments.json: - return execute_command_and_capture_output( - full_command, - extra_environment=environment.make_environment(config), - borg_local_path=local_path, - ) - else: - execute_command( - full_command, - output_log_level=logging.ANSWER, - borg_local_path=local_path, - extra_environment=environment.make_environment(config), - ) + return json_info + + flags.warn_for_aggressive_archive_flags(json_command, json_info) + + execute_command( + main_command, + output_log_level=logging.ANSWER, + borg_local_path=local_path, + extra_environment=environment.make_environment(config), + ) diff --git a/borgmatic/borg/rlist.py b/borgmatic/borg/rlist.py index b6ceca31..54b45b34 100644 --- a/borgmatic/borg/rlist.py +++ b/borgmatic/borg/rlist.py @@ -1,3 +1,4 @@ +import argparse import logging import borgmatic.logger @@ -137,15 +138,28 @@ def list_repository( local_path, remote_path, ) + json_command = make_rlist_command( + repository_path, + config, + local_borg_version, + argparse.Namespace(**dict(rlist_arguments.__dict__, json=True)), + global_arguments, + local_path, + remote_path, + ) + + json_listing = execute_command_and_capture_output( + json_command, extra_environment=borg_environment, borg_local_path=local_path + ) if rlist_arguments.json: - return execute_command_and_capture_output( - main_command, extra_environment=borg_environment, borg_local_path=local_path - ) - else: - execute_command( - main_command, - output_log_level=logging.ANSWER, - borg_local_path=local_path, - extra_environment=borg_environment, - ) + return json_listing + + flags.warn_for_aggressive_archive_flags(json_command, json_listing) + + execute_command( + main_command, + output_log_level=logging.ANSWER, + borg_local_path=local_path, + extra_environment=borg_environment, + ) diff --git a/tests/integration/borg/test_commands.py b/tests/integration/borg/test_commands.py index 9a121215..c46f6459 100644 --- a/tests/integration/borg/test_commands.py +++ b/tests/integration/borg/test_commands.py @@ -32,6 +32,9 @@ def assert_command_does_not_duplicate_flags(command, *args, **kwargs): flag_name: 1 for flag_name in flag_counts }, f"Duplicate flags found in: {' '.join(command)}" + if '--json' in command: + return '{}' + def fuzz_argument(arguments, argument_name): ''' diff --git a/tests/unit/borg/test_flags.py b/tests/unit/borg/test_flags.py index ba6cf2ea..dde9f7a5 100644 --- a/tests/unit/borg/test_flags.py +++ b/tests/unit/borg/test_flags.py @@ -164,3 +164,53 @@ def test_make_match_archives_flags_makes_flags_with_globs( ) == expected_result ) + + +def test_warn_for_aggressive_archive_flags_without_archive_flags_bails(): + flexmock(module.logger).should_receive('warning').never() + + module.warn_for_aggressive_archive_flags(('borg', '--do-stuff'), '{}') + + +def test_warn_for_aggressive_archive_flags_with_glob_archives_and_zero_archives_warns(): + flexmock(module.logger).should_receive('warning').twice() + + module.warn_for_aggressive_archive_flags( + ('borg', '--glob-archives', 'foo*'), '{"archives": []}' + ) + + +def test_warn_for_aggressive_archive_flags_with_match_archives_and_zero_archives_warns(): + flexmock(module.logger).should_receive('warning').twice() + + module.warn_for_aggressive_archive_flags( + ('borg', '--match-archives', 'foo*'), '{"archives": []}' + ) + + +def test_warn_for_aggressive_archive_flags_with_glob_archives_and_one_archive_does_not_warn(): + flexmock(module.logger).should_receive('warning').never() + + module.warn_for_aggressive_archive_flags( + ('borg', '--glob-archives', 'foo*'), '{"archives": [{"name": "foo"]}' + ) + + +def test_warn_for_aggressive_archive_flags_with_match_archives_and_one_archive_does_not_warn(): + flexmock(module.logger).should_receive('warning').never() + + module.warn_for_aggressive_archive_flags( + ('borg', '--match-archives', 'foo*'), '{"archives": [{"name": "foo"]}' + ) + + +def test_warn_for_aggressive_archive_flags_with_glob_archives_and_invalid_json_does_not_warn(): + flexmock(module.logger).should_receive('warning').never() + + module.warn_for_aggressive_archive_flags(('borg', '--glob-archives', 'foo*'), '{"archives": [}') + + +def test_warn_for_aggressive_archive_flags_with_glob_archives_and_json_missing_archives_does_not_warn(): + flexmock(module.logger).should_receive('warning').never() + + module.warn_for_aggressive_archive_flags(('borg', '--glob-archives', 'foo*'), '{}') diff --git a/tests/unit/borg/test_info.py b/tests/unit/borg/test_info.py index 107c4863..7c644436 100644 --- a/tests/unit/borg/test_info.py +++ b/tests/unit/borg/test_info.py @@ -8,224 +8,178 @@ from borgmatic.borg import info as module from ..test_verbosity import insert_logging_mock -def test_display_archives_info_calls_borg_with_parameters(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER +def test_make_info_command_constructs_borg_info_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( None, None, '2.3.4' ).and_return(()) flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--repo', 'repo') -def test_display_archives_info_with_log_info_calls_borg_with_info_parameter(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_with_log_info_passes_through_to_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( None, None, '2.3.4' ).and_return(()) flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--info', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) insert_logging_mock(logging.INFO) - module.display_archives_info( + + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--info', '--repo', 'repo') -def test_display_archives_info_with_log_info_and_json_suppresses_most_borg_output(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_with_log_info_and_json_omits_borg_logging_flags(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( None, None, '2.3.4' ).and_return(()) 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_and_capture_output').with_args( - ('borg', 'info', '--json', '--repo', 'repo'), - extra_environment=None, - borg_local_path='borg', - ).and_return('[]') - insert_logging_mock(logging.INFO) - json_output = module.display_archives_info( + + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=True, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) - assert json_output == '[]' + assert command == ('borg', 'info', '--json', '--repo', 'repo') -def test_display_archives_info_with_log_debug_calls_borg_with_debug_parameter(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER +def test_make_info_command_with_log_debug_passes_through_to_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( None, None, '2.3.4' ).and_return(()) flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--debug', '--show-rc', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) insert_logging_mock(logging.DEBUG) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--debug', '--show-rc', '--repo', 'repo') -def test_display_archives_info_with_log_debug_and_json_suppresses_most_borg_output(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_with_log_debug_and_json_omits_borg_logging_flags(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( None, None, '2.3.4' ).and_return(()) 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_and_capture_output').with_args( - ('borg', 'info', '--json', '--repo', 'repo'), - extra_environment=None, - borg_local_path='borg', - ).and_return('[]') - insert_logging_mock(logging.DEBUG) - json_output = module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=True, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) - assert json_output == '[]' + assert command == ('borg', 'info', '--json', '--repo', 'repo') -def test_display_archives_info_with_json_calls_borg_with_json_parameter(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER +def test_make_info_command_with_json_passes_through_to_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( None, None, '2.3.4' ).and_return(()) 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_and_capture_output').with_args( - ('borg', 'info', '--json', '--repo', 'repo'), - extra_environment=None, - borg_local_path='borg', - ).and_return('[]') - json_output = module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=True, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) - assert json_output == '[]' + assert command == ('borg', 'info', '--json', '--repo', 'repo') -def test_display_archives_info_with_archive_calls_borg_with_match_archives_parameter(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER +def test_make_info_command_with_archive_uses_match_archives_flags(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( 'archive', None, '2.3.4' ).and_return(('--match-archives', 'archive')) flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--match-archives', 'archive', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive='archive', json=False, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--match-archives', 'archive', '--repo', 'repo') -def test_display_archives_info_with_local_path_calls_borg_via_local_path(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_with_local_path_passes_through_to_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( None, None, '2.3.4' ).and_return(()) flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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( - ('borg1', 'info', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg1', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives=None), local_path='borg1', + remote_path=None, ) + command == ('borg1', 'info', '--repo', 'repo') -def test_display_archives_info_with_remote_path_calls_borg_with_remote_path_parameters(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_with_remote_path_passes_through_to_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_flags').with_args( 'remote-path', 'borg1' @@ -235,27 +189,21 @@ def test_display_archives_info_with_remote_path_calls_borg_with_remote_path_para ).and_return(()) flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--remote-path', 'borg1', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives=None), + local_path='borg', remote_path='borg1', ) + assert command == ('borg', 'info', '--remote-path', 'borg1', '--repo', 'repo') -def test_display_archives_info_with_log_json_calls_borg_with_log_json_parameters(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_with_log_json_passes_through_to_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_flags').with_args('log-json', True).and_return( ('--log-json',) @@ -265,26 +213,21 @@ def test_display_archives_info_with_log_json_calls_borg_with_log_json_parameters ).and_return(()) flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--log-json', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=True), info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--log-json', '--repo', 'repo') -def test_display_archives_info_with_lock_wait_calls_borg_with_lock_wait_parameters(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_with_lock_wait_passes_through_to_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_flags').with_args('lock-wait', 5).and_return( ('--lock-wait', '5') @@ -295,26 +238,21 @@ def test_display_archives_info_with_lock_wait_calls_borg_with_lock_wait_paramete flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) flexmock(module.flags).should_receive('make_repository_flags').and_return(('--repo', 'repo')) config = {'lock_wait': 5} - flexmock(module.environment).should_receive('make_environment') - flexmock(module).should_receive('execute_command').with_args( - ('borg', 'info', '--lock-wait', '5', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config=config, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--lock-wait', '5', '--repo', 'repo') -def test_display_archives_info_transforms_prefix_into_match_archives_parameters(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_transforms_prefix_into_match_archives_flags(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_flags').with_args( 'match-archives', 'sh:foo*' @@ -324,26 +262,21 @@ def test_display_archives_info_transforms_prefix_into_match_archives_parameters( ).and_return(()) flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--match-archives', 'sh:foo*', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix='foo'), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--match-archives', 'sh:foo*', '--repo', 'repo') -def test_display_archives_info_prefers_prefix_over_archive_name_format(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_prefers_prefix_over_archive_name_format(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_flags').with_args( 'match-archives', 'sh:foo*' @@ -353,52 +286,42 @@ def test_display_archives_info_prefers_prefix_over_archive_name_format(): ).and_return(()) flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--match-archives', 'sh:foo*', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={'archive_name_format': 'bar-{now}'}, # noqa: FS003 local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix='foo'), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--match-archives', 'sh:foo*', '--repo', 'repo') -def test_display_archives_info_transforms_archive_name_format_into_match_archives_parameters(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_transforms_archive_name_format_into_match_archives_flags(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( None, 'bar-{now}', '2.3.4' # noqa: FS003 ).and_return(('--match-archives', 'sh:bar-*')) flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--match-archives', 'sh:bar-*', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={'archive_name_format': 'bar-{now}'}, # noqa: FS003 local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--match-archives', 'sh:bar-*', '--repo', 'repo') -def test_display_archives_with_match_archives_option_calls_borg_with_match_archives_parameter(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_with_match_archives_option_passes_through_to_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( 'sh:foo-*', 'bar-{now}', '2.3.4' # noqa: FS003 @@ -406,14 +329,8 @@ def test_display_archives_with_match_archives_option_calls_borg_with_match_archi flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--match-archives', 'sh:foo-*', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={ 'archive_name_format': 'bar-{now}', # noqa: FS003 @@ -422,12 +339,14 @@ def test_display_archives_with_match_archives_option_calls_borg_with_match_archi local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives=None), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--match-archives', 'sh:foo-*', '--repo', 'repo') -def test_display_archives_with_match_archives_flag_calls_borg_with_match_archives_parameter(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_with_match_archives_flag_passes_through_to_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( 'sh:foo-*', 'bar-{now}', '2.3.4' # noqa: FS003 @@ -435,26 +354,22 @@ def test_display_archives_with_match_archives_flag_calls_borg_with_match_archive flexmock(module.flags).should_receive('make_flags_from_arguments').and_return(()) 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', '--match-archives', 'sh:foo-*', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={'archive_name_format': 'bar-{now}'}, # noqa: FS003 local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives='sh:foo-*'), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', '--match-archives', 'sh:foo-*', '--repo', 'repo') + @pytest.mark.parametrize('argument_name', ('sort_by', 'first', 'last')) -def test_display_archives_info_passes_through_arguments_to_borg(argument_name): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER +def test_make_info_command_passes_arguments_through_to_command(argument_name): flag_name = f"--{argument_name.replace('_', ' ')}" flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( @@ -465,14 +380,8 @@ def test_display_archives_info_passes_through_arguments_to_borg(argument_name): ) 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', flag_name, 'value', '--repo', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) - module.display_archives_info( + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', @@ -480,12 +389,14 @@ def test_display_archives_info_passes_through_arguments_to_borg(argument_name): info_arguments=flexmock( archive=None, json=False, prefix=None, match_archives=None, **{argument_name: 'value'} ), + local_path='borg', + remote_path=None, ) + assert command == ('borg', 'info', flag_name, 'value', '--repo', 'repo') -def test_display_archives_info_with_date_based_matching_calls_borg_with_date_based_flags(): - flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER + +def test_make_info_command_with_date_based_matching_passes_through_to_command(): flexmock(module.flags).should_receive('make_flags').and_return(()) flexmock(module.flags).should_receive('make_match_archives_flags').with_args( None, None, '2.3.4' @@ -494,26 +405,6 @@ def test_display_archives_info_with_date_based_matching_calls_borg_with_date_bas ('--newer', '1d', '--newest', '1y', '--older', '1m', '--oldest', '1w') ) 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', - '--newer', - '1d', - '--newest', - '1y', - '--older', - '1m', - '--oldest', - '1w', - '--repo', - 'repo', - ), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ) info_arguments = flexmock( archive=None, json=False, @@ -524,10 +415,66 @@ def test_display_archives_info_with_date_based_matching_calls_borg_with_date_bas older='1m', oldest='1w', ) - module.display_archives_info( + + command = module.make_info_command( repository_path='repo', config={}, local_borg_version='2.3.4', global_arguments=flexmock(log_json=False), info_arguments=info_arguments, + local_path='borg', + remote_path=None, + ) + + assert command == ( + 'borg', + 'info', + '--newer', + '1d', + '--newest', + '1y', + '--older', + '1m', + '--oldest', + '1w', + '--repo', + 'repo', + ) + + +def test_display_archives_info_calls_two_commands(): + flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') + flexmock(module).should_receive('make_info_command') + flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command_and_capture_output').once() + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags') + flexmock(module).should_receive('execute_command').once() + + module.display_archives_info( + repository_path='repo', + config={}, + local_borg_version='2.3.4', + global_arguments=flexmock(log_json=False), + info_arguments=flexmock(archive=None, json=False, prefix=None, match_archives=None), + ) + + +def test_display_archives_info_with_json_calls_json_command_only(): + flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') + flexmock(module).should_receive('make_info_command') + flexmock(module.environment).should_receive('make_environment') + json_output = flexmock() + flexmock(module).should_receive('execute_command_and_capture_output').and_return(json_output) + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags').never() + flexmock(module).should_receive('execute_command').never() + + assert ( + module.display_archives_info( + repository_path='repo', + config={}, + local_borg_version='2.3.4', + global_arguments=flexmock(log_json=False), + info_arguments=flexmock(archive=None, json=True, prefix=None, match_archives=None), + ) + == json_output ) diff --git a/tests/unit/borg/test_rinfo.py b/tests/unit/borg/test_rinfo.py index ee5e5c24..41bfd2eb 100644 --- a/tests/unit/borg/test_rinfo.py +++ b/tests/unit/borg/test_rinfo.py @@ -18,6 +18,12 @@ def test_display_repository_info_calls_borg_with_flags(): ) ) flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command_and_capture_output').with_args( + ('borg', 'rinfo', '--json', '--repo', 'repo'), + borg_local_path='borg', + extra_environment=None, + ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags') flexmock(module).should_receive('execute_command').with_args( ('borg', 'rinfo', '--repo', 'repo'), output_log_level=module.borgmatic.logger.ANSWER, @@ -40,6 +46,12 @@ def test_display_repository_info_without_borg_features_calls_borg_with_info_sub_ flexmock(module.feature).should_receive('available').and_return(False) flexmock(module.flags).should_receive('make_repository_flags').and_return(('repo',)) flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command_and_capture_output').with_args( + ('borg', 'rinfo', '--json', 'repo'), + borg_local_path='borg', + extra_environment=None, + ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags') flexmock(module).should_receive('execute_command').with_args( ('borg', 'info', 'repo'), output_log_level=module.borgmatic.logger.ANSWER, @@ -67,6 +79,12 @@ def test_display_repository_info_with_log_info_calls_borg_with_info_flag(): ) ) flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command_and_capture_output').with_args( + ('borg', 'rinfo', '--info', '--json', '--repo', 'repo'), + borg_local_path='borg', + extra_environment=None, + ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags') flexmock(module).should_receive('execute_command').with_args( ('borg', 'rinfo', '--info', '--repo', 'repo'), output_log_level=module.borgmatic.logger.ANSWER, @@ -99,6 +117,7 @@ def test_display_repository_info_with_log_info_and_json_suppresses_most_borg_out extra_environment=None, borg_local_path='borg', ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags').never() insert_logging_mock(logging.INFO) json_output = module.display_repository_info( @@ -123,6 +142,12 @@ def test_display_repository_info_with_log_debug_calls_borg_with_debug_flag(): ) ) flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command_and_capture_output').with_args( + ('borg', 'rinfo', '--debug', '--show-rc', '--json', '--repo', 'repo'), + borg_local_path='borg', + extra_environment=None, + ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags') flexmock(module).should_receive('execute_command').with_args( ('borg', 'rinfo', '--debug', '--show-rc', '--repo', 'repo'), output_log_level=module.borgmatic.logger.ANSWER, @@ -156,6 +181,7 @@ def test_display_repository_info_with_log_debug_and_json_suppresses_most_borg_ou extra_environment=None, borg_local_path='borg', ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags').never() insert_logging_mock(logging.DEBUG) json_output = module.display_repository_info( @@ -185,6 +211,7 @@ def test_display_repository_info_with_json_calls_borg_with_json_flag(): extra_environment=None, borg_local_path='borg', ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags').never() json_output = module.display_repository_info( repository_path='repo', @@ -208,6 +235,12 @@ def test_display_repository_info_with_local_path_calls_borg_via_local_path(): ) ) flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command_and_capture_output').with_args( + ('borg1', 'rinfo', '--json', '--repo', 'repo'), + extra_environment=None, + borg_local_path='borg', + ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags') flexmock(module).should_receive('execute_command').with_args( ('borg1', 'rinfo', '--repo', 'repo'), output_log_level=module.borgmatic.logger.ANSWER, @@ -236,6 +269,12 @@ def test_display_repository_info_with_remote_path_calls_borg_with_remote_path_fl ) ) flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command_and_capture_output').with_args( + ('borg', 'rinfo', '--remote-path', 'borg1', '--json', '--repo', 'repo'), + extra_environment=None, + borg_local_path='borg', + ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags') flexmock(module).should_receive('execute_command').with_args( ('borg', 'rinfo', '--remote-path', 'borg1', '--repo', 'repo'), output_log_level=module.borgmatic.logger.ANSWER, @@ -264,6 +303,12 @@ def test_display_repository_info_with_log_json_calls_borg_with_log_json_flags(): ) ) flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command_and_capture_output').with_args( + ('borg', 'rinfo', '--log-json', '--json', '--repo', 'repo'), + extra_environment=None, + borg_local_path='borg', + ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags') flexmock(module).should_receive('execute_command').with_args( ('borg', 'rinfo', '--log-json', '--repo', 'repo'), output_log_level=module.borgmatic.logger.ANSWER, @@ -292,6 +337,12 @@ def test_display_repository_info_with_lock_wait_calls_borg_with_lock_wait_flags( ) ) flexmock(module.environment).should_receive('make_environment') + flexmock(module).should_receive('execute_command_and_capture_output').with_args( + ('borg', 'rinfo', '--lock-wait', '5', '--json', '--repo', 'repo'), + extra_environment=None, + borg_local_path='borg', + ).and_return('[]') + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags') flexmock(module).should_receive('execute_command').with_args( ('borg', 'rinfo', '--lock-wait', '5', '--repo', 'repo'), output_log_level=module.borgmatic.logger.ANSWER, diff --git a/tests/unit/borg/test_rlist.py b/tests/unit/borg/test_rlist.py index a61a837e..0cd7736f 100644 --- a/tests/unit/borg/test_rlist.py +++ b/tests/unit/borg/test_rlist.py @@ -559,66 +559,39 @@ def test_make_rlist_command_with_match_archives_calls_borg_with_match_archives_f assert command == ('borg', 'list', '--match-archives', 'foo-*', 'repo') -def test_list_repository_calls_borg_with_flags(): +def test_list_repository_calls_two_commands(): flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER - rlist_arguments = argparse.Namespace(json=False) - global_arguments = flexmock() - - flexmock(module.feature).should_receive('available').and_return(False) - flexmock(module).should_receive('make_rlist_command').with_args( - repository_path='repo', - config={}, - local_borg_version='1.2.3', - rlist_arguments=rlist_arguments, - global_arguments=global_arguments, - local_path='borg', - remote_path=None, - ).and_return(('borg', 'rlist', 'repo')) + flexmock(module).should_receive('make_rlist_command') flexmock(module.environment).should_receive('make_environment') - flexmock(module).should_receive('execute_command').with_args( - ('borg', 'rlist', 'repo'), - output_log_level=module.borgmatic.logger.ANSWER, - borg_local_path='borg', - extra_environment=None, - ).once() + flexmock(module).should_receive('execute_command_and_capture_output').once() + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags') + flexmock(module).should_receive('execute_command').once() module.list_repository( repository_path='repo', config={}, local_borg_version='1.2.3', - rlist_arguments=rlist_arguments, - global_arguments=global_arguments, + rlist_arguments=argparse.Namespace(json=False), + global_arguments=flexmock(), ) -def test_list_repository_with_json_returns_borg_output(): +def test_list_repository_with_json_calls_json_command_only(): flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels') - flexmock(module.logging).ANSWER = module.borgmatic.logger.ANSWER - rlist_arguments = argparse.Namespace(json=True) - global_arguments = flexmock() - json_output = flexmock() - - flexmock(module.feature).should_receive('available').and_return(False) - flexmock(module).should_receive('make_rlist_command').with_args( - repository_path='repo', - config={}, - local_borg_version='1.2.3', - rlist_arguments=rlist_arguments, - global_arguments=global_arguments, - local_path='borg', - remote_path=None, - ).and_return(('borg', 'rlist', 'repo')) + flexmock(module).should_receive('make_rlist_command') flexmock(module.environment).should_receive('make_environment') + json_output = flexmock() flexmock(module).should_receive('execute_command_and_capture_output').and_return(json_output) + flexmock(module.flags).should_receive('warn_for_aggressive_archive_flags').never() + flexmock(module).should_receive('execute_command').never() assert ( module.list_repository( repository_path='repo', config={}, local_borg_version='1.2.3', - rlist_arguments=rlist_arguments, - global_arguments=global_arguments, + rlist_arguments=argparse.Namespace(json=True), + global_arguments=flexmock(), ) == json_output )