From 0fbdf8d860ec47257f23621397396ccfa563927a Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 6 Apr 2023 09:31:24 +0530 Subject: [PATCH 1/3] feat: add logfile name to hook context for interpolation --- borgmatic/commands/borgmatic.py | 6 ++++++ tests/unit/commands/test_borgmatic.py | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 22aba0b1..b6d3c4bb 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -269,6 +269,12 @@ def run_actions( 'repositories': ','.join([repo['path'] for repo in location['repositories']]), } + try: + log_file = global_arguments.log_file + hook_context['log_file'] = log_file + except AttributeError: + pass + command.execute_hook( hooks.get('before_actions'), hooks.get('umask'), diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index 11e879df..44a08590 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -422,6 +422,26 @@ def test_run_actions_runs_rcreate(): ) ) +def test_run_actions_adds_log_file_to_hook_context(): + flexmock(module).should_receive('add_custom_log_levels') + flexmock(module.command).should_receive('execute_hook') + flexmock(borgmatic.actions.rcreate).should_receive('run_rcreate').once() + + tuple( + module.run_actions( + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'rcreate': flexmock()}, + config_filename=flexmock(), + location={'repositories': []}, + storage=flexmock(), + retention=flexmock(), + consistency=flexmock(), + hooks={}, + local_path=flexmock(), + remote_path=flexmock(), + local_borg_version=flexmock(), + repository={'path': 'repo'}, + ) + ) def test_run_actions_runs_transfer(): flexmock(module).should_receive('add_custom_log_levels') From 091d60c226f1d66f98f21252d6b072b62080b57c Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Thu, 6 Apr 2023 12:36:10 +0530 Subject: [PATCH 2/3] refactor and improve tests --- borgmatic/commands/borgmatic.py | 6 +-- tests/unit/commands/test_borgmatic.py | 53 +++++++++++++++++---------- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index b6d3c4bb..a4de94ba 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -269,11 +269,7 @@ def run_actions( 'repositories': ','.join([repo['path'] for repo in location['repositories']]), } - try: - log_file = global_arguments.log_file - hook_context['log_file'] = log_file - except AttributeError: - pass + hook_context['log_file'] = global_arguments.log_file if global_arguments.log_file else '' command.execute_hook( hooks.get('before_actions'), diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index 44a08590..63501853 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -408,7 +408,7 @@ def test_run_actions_runs_rcreate(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'rcreate': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'rcreate': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -425,11 +425,24 @@ def test_run_actions_runs_rcreate(): def test_run_actions_adds_log_file_to_hook_context(): flexmock(module).should_receive('add_custom_log_levels') flexmock(module.command).should_receive('execute_hook') - flexmock(borgmatic.actions.rcreate).should_receive('run_rcreate').once() + flexmock(borgmatic.actions.create).should_receive('run_create').with_args( + config_filename=flexmock(), + repository={'path': 'repo'}, + location={'repositories': []}, + storage=flexmock(), + hooks={}, + hook_context={'log_file': 'foo'}, + local_borg_version=flexmock(), + create_arguments=flexmock(), + global_arguments=flexmock(dry_run=False, log_file='foo'), + dry_run_label='', + local_path=flexmock(), + remote_path=flexmock(), + ).once() tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False, log_file='foo'), 'rcreate': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'create': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -450,7 +463,7 @@ def test_run_actions_runs_transfer(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'transfer': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'transfer': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -473,7 +486,7 @@ def test_run_actions_runs_create(): result = tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'create': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'create': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -496,7 +509,7 @@ def test_run_actions_runs_prune(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'prune': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'prune': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -518,7 +531,7 @@ def test_run_actions_runs_compact(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'compact': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'compact': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -541,7 +554,7 @@ def test_run_actions_runs_check_when_repository_enabled_for_checks(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'check': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'check': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -564,7 +577,7 @@ def test_run_actions_skips_check_when_repository_not_enabled_for_checks(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'check': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'check': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -586,7 +599,7 @@ def test_run_actions_runs_extract(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'extract': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'extract': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -608,7 +621,7 @@ def test_run_actions_runs_export_tar(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'export-tar': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'export-tar': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -630,7 +643,7 @@ def test_run_actions_runs_mount(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'mount': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'mount': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -652,7 +665,7 @@ def test_run_actions_runs_restore(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'restore': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'restore': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -675,7 +688,7 @@ def test_run_actions_runs_rlist(): result = tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'rlist': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'rlist': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -699,7 +712,7 @@ def test_run_actions_runs_list(): result = tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'list': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'list': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -723,7 +736,7 @@ def test_run_actions_runs_rinfo(): result = tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'rinfo': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'rinfo': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -747,7 +760,7 @@ def test_run_actions_runs_info(): result = tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'info': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'info': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -770,7 +783,7 @@ def test_run_actions_runs_break_lock(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'break-lock': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'break-lock': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -792,7 +805,7 @@ def test_run_actions_runs_borg(): tuple( module.run_actions( - arguments={'global': flexmock(dry_run=False), 'borg': flexmock()}, + arguments={'global': flexmock(dry_run=False, log_file='foo'), 'borg': flexmock()}, config_filename=flexmock(), location={'repositories': []}, storage=flexmock(), @@ -816,7 +829,7 @@ def test_run_actions_runs_multiple_actions_in_argument_order(): tuple( module.run_actions( arguments={ - 'global': flexmock(dry_run=False), + 'global': flexmock(dry_run=False, log_file='foo'), 'borg': flexmock(), 'restore': flexmock(), }, From 16d7131fb75b2aef9bd02c0af47607c86c209572 Mon Sep 17 00:00:00 2001 From: Divyansh Singh Date: Fri, 7 Apr 2023 01:00:38 +0530 Subject: [PATCH 3/3] refactor tests --- borgmatic/commands/borgmatic.py | 3 +-- tests/unit/commands/test_borgmatic.py | 24 ++++++++++++++---------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index a4de94ba..5d56accb 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -267,10 +267,9 @@ def run_actions( 'repository': repository_path, # Deprecated: For backwards compatibility with borgmatic < 1.6.0. 'repositories': ','.join([repo['path'] for repo in location['repositories']]), + 'log_file': global_arguments.log_file if global_arguments.log_file else '', } - hook_context['log_file'] = global_arguments.log_file if global_arguments.log_file else '' - command.execute_hook( hooks.get('before_actions'), hooks.get('umask'), diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py index 63501853..56f2332c 100644 --- a/tests/unit/commands/test_borgmatic.py +++ b/tests/unit/commands/test_borgmatic.py @@ -422,25 +422,27 @@ def test_run_actions_runs_rcreate(): ) ) + def test_run_actions_adds_log_file_to_hook_context(): flexmock(module).should_receive('add_custom_log_levels') flexmock(module.command).should_receive('execute_hook') + expected = flexmock() flexmock(borgmatic.actions.create).should_receive('run_create').with_args( - config_filename=flexmock(), + config_filename=object, repository={'path': 'repo'}, location={'repositories': []}, - storage=flexmock(), + storage=object, hooks={}, - hook_context={'log_file': 'foo'}, - local_borg_version=flexmock(), - create_arguments=flexmock(), - global_arguments=flexmock(dry_run=False, log_file='foo'), + hook_context={'repository': 'repo', 'repositories': '', 'log_file': 'foo'}, + local_borg_version=object, + create_arguments=object, + global_arguments=object, dry_run_label='', - local_path=flexmock(), - remote_path=flexmock(), - ).once() + local_path=object, + remote_path=object, + ).once().and_return(expected) - tuple( + result = tuple( module.run_actions( arguments={'global': flexmock(dry_run=False, log_file='foo'), 'create': flexmock()}, config_filename=flexmock(), @@ -455,6 +457,8 @@ def test_run_actions_adds_log_file_to_hook_context(): repository={'path': 'repo'}, ) ) + assert result == (expected,) + def test_run_actions_runs_transfer(): flexmock(module).should_receive('add_custom_log_levels')