Rework logging/verbosity system #90

Merged
witten merged 12 commits from :master into master 2018-09-08 20:53:38 +00:00
13 changed files with 93 additions and 134 deletions
Showing only changes of commit 3b5564babf - Show all commits

View File

@ -3,7 +3,6 @@ import os
import subprocess
from borgmatic.borg import extract
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
DEFAULT_CHECKS = ('repository', 'archives')
@ -74,8 +73,7 @@ def _make_check_flags(checks, check_last=None, prefix=None):
) + last_flags + prefix_flags
def check_archives(verbosity, repository, storage_config, consistency_config, local_path='borg',
remote_path=None):
def check_archives(repository, storage_config, consistency_config, local_path='borg', remote_path=None):
'''
Given a verbosity flag, a local or remote repository path, a storage config dict, a consistency
config dict, and a local/remote commands to run, check the contained Borg archives for
@ -91,10 +89,12 @@ def check_archives(verbosity, repository, storage_config, consistency_config, lo
remote_path_flags = ('--remote-path', remote_path) if remote_path else ()
lock_wait = storage_config.get('lock_wait', None)
lock_wait_flags = ('--lock-wait', str(lock_wait)) if lock_wait else ()
verbosity_flags = {
VERBOSITY_SOME: ('--info',),
VERBOSITY_LOTS: ('--debug',),
}.get(verbosity, ())
verbosity_flags = ()
if logger.isEnabledFor(logging.INFO):
verbosity_flags = ('--info',)
if logger.isEnabledFor(logging.DEBUG):
verbosity_flags = ('--debug',)
prefix = consistency_config.get('prefix')
full_command = (
@ -109,4 +109,4 @@ def check_archives(verbosity, repository, storage_config, consistency_config, lo
subprocess.check_call(full_command, stdout=stdout, stderr=subprocess.STDOUT)
if 'extract' in checks:
extract.extract_last_archive_dry_run(verbosity, repository, lock_wait, local_path, remote_path)
extract.extract_last_archive_dry_run(repository, lock_wait, local_path, remote_path)

View File

@ -5,8 +5,6 @@ import os
import subprocess
import tempfile
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
logger = logging.getLogger(__name__)
@ -105,7 +103,7 @@ def _make_exclude_flags(location_config, exclude_filename=None):
def create_archive(
verbosity, dry_run, repository, location_config, storage_config, local_path='borg', remote_path=None,
dry_run, repository, location_config, storage_config, local_path='borg', remote_path=None,
):
'''
Given vebosity/dry-run flags, a local or remote repository path, a location config dict, and a
@ -148,10 +146,8 @@ def create_archive(
+ (('--remote-path', remote_path) if remote_path else ())
+ (('--umask', str(umask)) if umask else ())
+ (('--lock-wait', str(lock_wait)) if lock_wait else ())
+ {
VERBOSITY_SOME: ('--info',) if dry_run else ('--info', '--stats',),
VERBOSITY_LOTS: ('--debug', '--list',) if dry_run else ('--debug', '--list', '--stats',),
}.get(verbosity, ())
+ (('--list', '--filter', 'AME', '--info', '--stats') if logger.isEnabledFor(logging.INFO) else ())
+ (('--debug', ) if logger.isEnabledFor(logging.DEBUG) else ())
+ (('--dry-run',) if dry_run else ())

You'll need to retain the logic that omits --stats during a dry run, because --stats and --dry-run can't be specified together. More info here: https://borgbackup.readthedocs.io/en/stable/usage/create.html#description

You'll need to retain the logic that omits `--stats` during a dry run, because `--stats` and `--dry-run` can't be specified together. More info here: https://borgbackup.readthedocs.io/en/stable/usage/create.html#description
Outdated
Review

Oh, wasn't aware of that, done.

Oh, wasn't aware of that, done.
)

For options which are mutually exclusive for DEBUG and INFO, like --debug --info, I let the latter option overwrite the first.

Does that mean Borg just ignores the --info and respects only the --debug flag?

> For options which are mutually exclusive for DEBUG and INFO, like --debug --info, I let the latter option overwrite the first. Does that mean Borg just ignores the `--info` and respects only the `--debug` flag?
Outdated
Review

Exactly. Unfortunately I am not sure if this is documented behavior of borg or python argparse. I think best will be to ask borg's mailing list. Even cooler would be to not call the borg executable, but use a python API. But that leads us too far... ;-)

Exactly. Unfortunately I am not sure if this is documented behavior of borg or python argparse. I think best will be to ask borg's mailing list. Even cooler would be to not call the borg executable, but use a python API. But that leads us too far... ;-)

If you wanted to avoid relying on the undocumented behavior (--debug overriding --info), one idea is that you could break out the construction of flags to a separate if/elif similar to the code elsewhere. For instance, something like:

 verbosity_flags = ()

 if logger.isEnabledFor(logging.DEBUG):
     verbosity_flags = ('--list', '--filter', 'AME', '--debug', '--show-rc')
 elif logger.isEnabledFor(logging.INFO):
     verbosity_flags = ('--list', '--filter', 'AME', '--info')

Another option would be to use getEffectiveLevel() instead of isEnabledFor() to sidestep falling through the hierarchy from DEBUG to INFO. That also has the side benefit of letting you keep things inline and non-imperative:

+ (('--list', '--filter', 'AME') if logger.isEnabledFor(logging.INFO) else ())
+ (('--info',) if logger.getEffectiveLevel() == logging.INFO else ())
+ (('--debug', '--show-rc') if logger.getEffectiveLevel() == logging.DEBUG else ())
If you wanted to avoid relying on the undocumented behavior (`--debug` overriding `--info`), one idea is that you could break out the construction of flags to a separate `if`/`elif` similar to the code elsewhere. For instance, something like: ```python verbosity_flags = () if logger.isEnabledFor(logging.DEBUG): verbosity_flags = ('--list', '--filter', 'AME', '--debug', '--show-rc') elif logger.isEnabledFor(logging.INFO): verbosity_flags = ('--list', '--filter', 'AME', '--info') ``` Another option would be to use `getEffectiveLevel()` instead of `isEnabledFor()` to sidestep falling through the hierarchy from `DEBUG` to `INFO`. That also has the side benefit of letting you keep things inline and non-imperative: ```python + (('--list', '--filter', 'AME') if logger.isEnabledFor(logging.INFO) else ()) + (('--info',) if logger.getEffectiveLevel() == logging.INFO else ()) + (('--debug', '--show-rc') if logger.getEffectiveLevel() == logging.DEBUG else ()) ```
Outdated
Review

I already thought about using getEffectiveLevel. I would use it only for --info. More generally: only for parameters that should not be enabled at a higher log level. --debug should also be enabled if the log level is even higher than debug (not used, but it should be possible).

I have addressed this in the recent commit.

I already thought about using `getEffectiveLevel`. I would use it *only* for `--info`. More generally: only for parameters that should not be enabled at a higher log level. `--debug` should also be enabled if the log level is even higher than debug (not used, but it should be possible). I have addressed this in the recent commit.

Looks great, thanks!

Looks great, thanks!

View File

@ -2,23 +2,23 @@ import logging
import sys
import subprocess
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
logger = logging.getLogger(__name__)
def extract_last_archive_dry_run(verbosity, repository, lock_wait=None, local_path='borg', remote_path=None):
def extract_last_archive_dry_run(repository, lock_wait=None, local_path='borg', remote_path=None):
'''
Perform an extraction dry-run of just the most recent archive. If there are no archives, skip
the dry-run.
'''
remote_path_flags = ('--remote-path', remote_path) if remote_path else ()
lock_wait_flags = ('--lock-wait', str(lock_wait)) if lock_wait else ()
verbosity_flags = {
VERBOSITY_SOME: ('--info',),
VERBOSITY_LOTS: ('--debug',),
}.get(verbosity, ())
if logger.isEnabledFor(logging.DEBUG):
verbosity_flags = ('--debug',)
elif logger.isEnabledFor(logging.INFO):
verbosity_flags = ('--info',)
else:
verbosity_flags = ()
full_list_command = (
local_path, 'list',
@ -32,7 +32,7 @@ def extract_last_archive_dry_run(verbosity, repository, lock_wait=None, local_pa
if not last_archive_name:
return
list_flag = ('--list',) if verbosity == VERBOSITY_LOTS else ()
list_flag = ('--list',) if logger.isEnabledFor(logging.DEBUG) else ()
full_extract_command = (
local_path, 'extract',
'--dry-run',

View File

@ -1,14 +1,11 @@
import logging
import subprocess
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
logger = logging.getLogger(__name__)
def display_archives_info(verbosity, repository, storage_config, local_path='borg',
remote_path=None):
def display_archives_info(repository, storage_config, local_path='borg', remote_path=None):
'''
Given a verbosity flag, a local or remote repository path, and a storage config dict,
display summary information for Borg archives in the repository.
@ -19,10 +16,8 @@ def display_archives_info(verbosity, repository, storage_config, local_path='bor
(local_path, 'info', repository)
+ (('--remote-path', remote_path) if remote_path else ())
+ (('--lock-wait', str(lock_wait)) if lock_wait else ())
+ {
VERBOSITY_SOME: ('--info',),
VERBOSITY_LOTS: ('--debug',),
}.get(verbosity, ())
+ (('--info',) if logger.isEnabledFor(logging.INFO) else ())
+ (('--debug',) if logger.isEnabledFor(logging.DEBUG) else ())
)
logger.debug(' '.join(full_command))

View File

@ -1,13 +1,11 @@
import logging
import subprocess
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
logger = logging.getLogger(__name__)
def list_archives(verbosity, repository, storage_config, local_path='borg', remote_path=None):
def list_archives(repository, storage_config, local_path='borg', remote_path=None):
'''
Given a verbosity flag, a local or remote repository path, and a storage config dict,
list Borg archives in the repository.
@ -18,11 +16,8 @@ def list_archives(verbosity, repository, storage_config, local_path='borg', remo
(local_path, 'list', repository)
+ (('--remote-path', remote_path) if remote_path else ())
+ (('--lock-wait', str(lock_wait)) if lock_wait else ())
+ {
VERBOSITY_SOME: ('--info',),
VERBOSITY_LOTS: ('--debug',),
}.get(verbosity, ())
+ (('--info',) if logger.isEnabledFor(logging.INFO) else ())
+ (('--debug',) if logger.isEnabledFor(logging.DEBUG) else ())
)
logger.debug(' '.join(full_command))
subprocess.check_call(full_command)

View File

@ -1,7 +1,6 @@
import logging
import subprocess
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
logger = logging.getLogger(__name__)
@ -32,10 +31,9 @@ def _make_prune_flags(retention_config):
)
def prune_archives(verbosity, dry_run, repository, storage_config, retention_config,
local_path='borg', remote_path=None):
def prune_archives(dry_run, repository, storage_config, retention_config, local_path='borg', remote_path=None):
'''
Given verbosity/dry-run flags, a local or remote repository path, a storage config dict, and a
Given dry-run flag, a local or remote repository path, a storage config dict, and a
retention config dict, prune Borg archives according to the retention policy specified in that
configuration.
'''
@ -54,10 +52,9 @@ def prune_archives(verbosity, dry_run, repository, storage_config, retention_con
+ (('--remote-path', remote_path) if remote_path else ())
+ (('--umask', str(umask)) if umask else ())
+ (('--lock-wait', str(lock_wait)) if lock_wait else ())
+ {
VERBOSITY_SOME: ('--info', '--stats',),
VERBOSITY_LOTS: ('--debug', '--stats', '--list'),
}.get(verbosity, ())
+ (('--stats',) if logger.isEnabledFor(logging.INFO) else ())
+ (('--info',) if logger.isEnabledFor(logging.INFO) else ())

Any reason not to combine these two lines?

Any reason not to combine these two lines?
Outdated
Review

Not really. Done.

Not really. Done.
+ (('--debug', '--list') if logger.isEnabledFor(logging.DEBUG) else ())
+ (('--dry-run',) if dry_run else ())
)

View File

@ -10,7 +10,7 @@ from borgmatic.borg import check as borg_check, create as borg_create, prune as
from borgmatic.commands import hook
from borgmatic.config import collect, convert, validate
from borgmatic.signals import configure_signals
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS, verbosity_to_log_level
from borgmatic.verbosity import verbosity_to_log_level
logger = logging.getLogger(__name__)
@ -83,7 +83,7 @@ def parse_arguments(*arguments):
parser.add_argument(
'-v', '--verbosity',
type=int,
help='Display verbose progress (1 for some, 2 for lots)',
help='Display verbose progress (1 for INFO, 2 for DEBUG)',
)
args = parser.parse_args(arguments)
@ -123,7 +123,6 @@ def run_configuration(config_filename, args): # pragma: no cover
if args.prune:
logger.info('{}: Pruning archives{}'.format(repository, dry_run_label))

Any reason for the switch to debug here? I think the idea with it being info is that you could easily see what borgmatic is doing with just the first (-v 1) verbosity level. Especially useful if you've got multiple configuration files and you want to see what's being done for each.

Any reason for the switch to `debug` here? I think the idea with it being `info` is that you could easily see what borgmatic is doing with just the first (`-v 1`) verbosity level. Especially useful if you've got multiple configuration files and you want to see what's being done for each.
Outdated
Review

Definitely a matter of taste here. Switched it back to info, as you proposed.

Definitely a matter of taste here. Switched it back to info, as you proposed.
borg_prune.prune_archives(
args.verbosity,
args.dry_run,
repository,
storage,
@ -134,7 +133,6 @@ def run_configuration(config_filename, args): # pragma: no cover
if args.create:
logger.info('{}: Creating archive{}'.format(repository, dry_run_label))
borg_create.create_archive(
args.verbosity,
args.dry_run,
repository,
location,
@ -145,7 +143,6 @@ def run_configuration(config_filename, args): # pragma: no cover
if args.check:
logger.info('{}: Running consistency checks'.format(repository))
borg_check.check_archives(
args.verbosity,
repository,
storage,
consistency,
@ -155,7 +152,6 @@ def run_configuration(config_filename, args): # pragma: no cover
if args.list:
logger.info('{}: Listing archives'.format(repository))
borg_list.list_archives(
args.verbosity,
repository,
storage,
local_path=local_path,
@ -164,7 +160,6 @@ def run_configuration(config_filename, args): # pragma: no cover
if args.info:
logger.info('{}: Displaying summary info for archives'.format(repository))
borg_info.display_archives_info(
args.verbosity,
repository,
storage,
local_path=local_path,

View File

@ -1,11 +1,10 @@
from subprocess import STDOUT
import sys
import logging, sys
from flexmock import flexmock
import pytest
from borgmatic.borg import check as module
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
def insert_subprocess_mock(check_call_command, **kwargs):
@ -17,6 +16,12 @@ def insert_subprocess_never():
subprocess = flexmock(module.subprocess)
subprocess.should_receive('check_call').never()
def insert_logging_mock(log_level):
logging = flexmock(module.logging.Logger)
logging.should_receive('isEnabledFor').replace_with(lambda x : x >= log_level)

Super minor: It'd be great to use a more descriptive variable than just x here. Similar elsewhere this pattern occurs.

Super minor: It'd be great to use a more descriptive variable than just `x` here. Similar elsewhere this pattern occurs.
Outdated
Review

Done.

Done.
def test_parse_checks_returns_them_as_tuple():
checks = module._parse_checks({'checks': ['foo', 'disabled', 'bar']})
@ -125,7 +130,6 @@ def test_check_archives_calls_borg_with_parameters(checks):
flexmock(module.os).should_receive('devnull')
module.check_archives(
verbosity=None,
repository='repo',
storage_config={},
consistency_config=consistency_config,
@ -142,7 +146,6 @@ def test_check_archives_with_extract_check_calls_extract_only():
insert_subprocess_never()
module.check_archives(
verbosity=None,
repository='repo',
storage_config={},
consistency_config=consistency_config,
@ -154,13 +157,13 @@ def test_check_archives_with_verbosity_some_calls_borg_with_info_parameter():
consistency_config = {'check_last': None}
flexmock(module).should_receive('_parse_checks').and_return(checks)
flexmock(module).should_receive('_make_check_flags').and_return(())
insert_logging_mock(logging.INFO)
insert_subprocess_mock(
('borg', 'check', 'repo', '--info'),
stdout=None, stderr=STDOUT,
)
module.check_archives(
verbosity=VERBOSITY_SOME,
repository='repo',
storage_config={},
consistency_config=consistency_config,
@ -172,13 +175,13 @@ def test_check_archives_with_verbosity_lots_calls_borg_with_debug_parameter():
consistency_config = {'check_last': None}
flexmock(module).should_receive('_parse_checks').and_return(checks)
flexmock(module).should_receive('_make_check_flags').and_return(())
insert_logging_mock(logging.DEBUG)
insert_subprocess_mock(
('borg', 'check', 'repo', '--debug'),
stdout=None, stderr=STDOUT,
)
module.check_archives(
verbosity=VERBOSITY_LOTS,
repository='repo',
storage_config={},
consistency_config=consistency_config,
@ -191,7 +194,6 @@ def test_check_archives_without_any_checks_bails():
insert_subprocess_never()
module.check_archives(
verbosity=None,
repository='repo',
storage_config={},
consistency_config=consistency_config,
@ -213,7 +215,6 @@ def test_check_archives_with_local_path_calls_borg_via_local_path():
flexmock(module.os).should_receive('devnull')
module.check_archives(
verbosity=None,
repository='repo',
storage_config={},
consistency_config=consistency_config,
@ -236,7 +237,6 @@ def test_check_archives_with_remote_path_calls_borg_with_remote_path_parameters(
flexmock(module.os).should_receive('devnull')
module.check_archives(
verbosity=None,
repository='repo',
storage_config={},
consistency_config=consistency_config,
@ -259,7 +259,6 @@ def test_check_archives_with_lock_wait_calls_borg_with_lock_wait_parameters():
flexmock(module.os).should_receive('devnull')
module.check_archives(
verbosity=None,
repository='repo',
storage_config={'lock_wait': 5},
consistency_config=consistency_config,
@ -283,7 +282,6 @@ def test_check_archives_with_retention_prefix():
flexmock(module.os).should_receive('devnull')
module.check_archives(
verbosity=None,
repository='repo',
storage_config={},
consistency_config=consistency_config,

View File

@ -1,9 +1,12 @@
import os
import logging, os
from flexmock import flexmock
from borgmatic.borg import create as module
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
def insert_logging_mock(log_level):
logging = flexmock(module.logging.Logger)
logging.should_receive('isEnabledFor').replace_with(lambda x : x >= log_level)
def test_initialize_environment_with_passcommand_should_set_environment():
@ -216,7 +219,6 @@ def test_create_archive_calls_borg_with_parameters():
insert_subprocess_mock(CREATE_COMMAND)
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -237,7 +239,6 @@ def test_create_archive_with_patterns_calls_borg_with_patterns():
insert_subprocess_mock(CREATE_COMMAND + pattern_flags)
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -258,7 +259,6 @@ def test_create_archive_with_exclude_patterns_calls_borg_with_excludes():
insert_subprocess_mock(CREATE_COMMAND + exclude_flags)
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -276,10 +276,10 @@ def test_create_archive_with_verbosity_some_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 + ('--info', '--stats',))
insert_subprocess_mock(CREATE_COMMAND + ('--list', '--filter', 'AME', '--info', '--stats',))
insert_logging_mock(logging.INFO)
module.create_archive(
verbosity=VERBOSITY_SOME,
dry_run=False,
repository='repo',
location_config={
@ -296,10 +296,10 @@ def test_create_archive_with_verbosity_lots_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 + ('--debug', '--list', '--stats'))
insert_subprocess_mock(CREATE_COMMAND + ('--list', '--filter', 'AME', '--info', '--stats', '--debug'))
insert_logging_mock(logging.DEBUG)
module.create_archive(
verbosity=VERBOSITY_LOTS,
dry_run=False,
repository='repo',
location_config={
@ -320,7 +320,6 @@ def test_create_archive_with_dry_run_calls_borg_with_dry_run_parameter():
insert_subprocess_mock(CREATE_COMMAND + ('--dry-run',))
module.create_archive(
verbosity=None,
dry_run=True,
repository='repo',
location_config={
@ -338,10 +337,10 @@ def test_create_archive_with_dry_run_and_verbosity_some_calls_borg_without_stats
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 + ('--info', '--dry-run'))
insert_subprocess_mock(CREATE_COMMAND + ('--list', '--filter', 'AME', '--info', '--stats', '--dry-run'))
insert_logging_mock(logging.INFO)
module.create_archive(
verbosity=VERBOSITY_SOME,
dry_run=True,
repository='repo',
location_config={
@ -359,10 +358,10 @@ def test_create_archive_with_dry_run_and_verbosity_lots_calls_borg_without_stats
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 + ('--debug', '--list', '--dry-run'))
insert_subprocess_mock(CREATE_COMMAND + ('--list', '--filter', 'AME', '--info', '--stats', '--debug', '--dry-run'))
insert_logging_mock(logging.DEBUG)
module.create_archive(
verbosity=VERBOSITY_LOTS,
dry_run=True,
repository='repo',
location_config={
@ -382,7 +381,6 @@ def test_create_archive_with_compression_calls_borg_with_compression_parameters(
insert_subprocess_mock(CREATE_COMMAND + ('--compression', 'rle'))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -402,7 +400,6 @@ def test_create_archive_with_remote_rate_limit_calls_borg_with_remote_ratelimit_
insert_subprocess_mock(CREATE_COMMAND + ('--remote-ratelimit', '100'))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -422,7 +419,6 @@ def test_create_archive_with_one_file_system_calls_borg_with_one_file_system_par
insert_subprocess_mock(CREATE_COMMAND + ('--one-file-system',))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -443,7 +439,6 @@ def test_create_archive_with_bsd_flags_true_calls_borg_without_nobsdflags_parame
insert_subprocess_mock(CREATE_COMMAND)
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -464,7 +459,6 @@ def test_create_archive_with_bsd_flags_false_calls_borg_with_nobsdflags_paramete
insert_subprocess_mock(CREATE_COMMAND + ('--nobsdflags',))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -485,7 +479,6 @@ def test_create_archive_with_files_cache_calls_borg_with_files_cache_parameters(
insert_subprocess_mock(CREATE_COMMAND + ('--files-cache', 'ctime,size'))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -506,7 +499,6 @@ def test_create_archive_with_local_path_calls_borg_via_local_path():
insert_subprocess_mock(('borg1',) + CREATE_COMMAND[1:])
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -527,7 +519,6 @@ def test_create_archive_with_remote_path_calls_borg_with_remote_path_parameters(
insert_subprocess_mock(CREATE_COMMAND + ('--remote-path', 'borg1'))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -548,7 +539,6 @@ def test_create_archive_with_umask_calls_borg_with_umask_parameters():
insert_subprocess_mock(CREATE_COMMAND + ('--umask', '740'))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -568,7 +558,6 @@ def test_create_archive_with_lock_wait_calls_borg_with_lock_wait_parameters():
insert_subprocess_mock(CREATE_COMMAND + ('--lock-wait', '5'))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -589,7 +578,6 @@ def test_create_archive_with_source_directories_glob_expands():
flexmock(module.glob).should_receive('glob').with_args('foo*').and_return(['foo', 'food'])
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -610,7 +598,6 @@ def test_create_archive_with_non_matching_source_directories_glob_passes_through
flexmock(module.glob).should_receive('glob').with_args('foo*').and_return([])
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -630,7 +617,6 @@ def test_create_archive_with_glob_calls_borg_with_expanded_directories():
insert_subprocess_mock(('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food'))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -650,7 +636,6 @@ def test_create_archive_with_archive_name_format_calls_borg_with_archive_name():
insert_subprocess_mock(('borg', 'create', 'repo::ARCHIVE_NAME', 'foo', 'bar'))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={
@ -672,7 +657,6 @@ def test_create_archive_with_archive_name_format_accepts_borg_placeholders():
insert_subprocess_mock(('borg', 'create', 'repo::Documents_{hostname}-{now}', 'foo', 'bar'))
module.create_archive(
verbosity=None,
dry_run=False,
repository='repo',
location_config={

View File

@ -1,4 +1,4 @@
import sys
import logging, sys
from flexmock import flexmock
@ -21,6 +21,11 @@ def insert_subprocess_check_output_mock(check_output_command, result, **kwargs):
subprocess.should_receive('check_output').with_args(check_output_command, **kwargs).and_return(result).once()
def insert_logging_mock(log_level):
logging = flexmock(module.logging.Logger)
logging.should_receive('isEnabledFor').replace_with(lambda x : x >= log_level)

Given that this occurs several times, you could factor it out into a common test file. For instance, the existing borgmatic/tests/unit/test_verbosity.py might be a good candidate.

Given that this occurs several times, you could factor it out into a common test file. For instance, the existing `borgmatic/tests/unit/test_verbosity.py` might be a good candidate.
Outdated
Review

Yeah, already had this on my agenda. Done now.

Yeah, already had this on my agenda. Done now.
def test_extract_last_archive_dry_run_should_call_borg_with_last_archive():
flexmock(sys.stdout).encoding = 'utf-8'
insert_subprocess_check_output_mock(
@ -32,7 +37,6 @@ def test_extract_last_archive_dry_run_should_call_borg_with_last_archive():
)
module.extract_last_archive_dry_run(
verbosity=None,
repository='repo',
lock_wait=None,
)
@ -47,7 +51,6 @@ def test_extract_last_archive_dry_run_without_any_archives_should_bail():
insert_subprocess_never()
module.extract_last_archive_dry_run(
verbosity=None,
repository='repo',
lock_wait=None,
)
@ -62,9 +65,9 @@ def test_extract_last_archive_dry_run_with_verbosity_some_should_call_borg_with_
insert_subprocess_mock(
('borg', 'extract', '--dry-run', 'repo::archive2', '--info'),
)
insert_logging_mock(logging.INFO)
module.extract_last_archive_dry_run(
verbosity=VERBOSITY_SOME,
repository='repo',
lock_wait=None,
)
@ -79,9 +82,9 @@ def test_extract_last_archive_dry_run_with_verbosity_lots_should_call_borg_with_
insert_subprocess_mock(
('borg', 'extract', '--dry-run', 'repo::archive2', '--debug', '--list'),
)
insert_logging_mock(logging.DEBUG)
module.extract_last_archive_dry_run(
verbosity=VERBOSITY_LOTS,
repository='repo',
lock_wait=None,
)
@ -98,7 +101,6 @@ def test_extract_last_archive_dry_run_should_call_borg_via_local_path():
)
module.extract_last_archive_dry_run(
verbosity=None,
repository='repo',
lock_wait=None,
local_path='borg1',
@ -116,7 +118,6 @@ def test_extract_last_archive_dry_run_should_call_borg_with_remote_path_paramete
)
module.extract_last_archive_dry_run(
verbosity=None,
repository='repo',
lock_wait=None,
remote_path='borg1',
@ -134,7 +135,6 @@ def test_extract_last_archive_dry_run_should_call_borg_with_lock_wait_parameters
)
module.extract_last_archive_dry_run(
verbosity=None,
repository='repo',
lock_wait=5,
)

View File

@ -1,15 +1,19 @@
import logging
from collections import OrderedDict
from flexmock import flexmock
from borgmatic.borg import info as module
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
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 insert_logging_mock(log_level):
logging = flexmock(module.logging.Logger)
logging.should_receive('isEnabledFor').replace_with(lambda x : x >= log_level)
INFO_COMMAND = ('borg', 'info', 'repo')
@ -18,7 +22,6 @@ def test_display_archives_info_calls_borg_with_parameters():
insert_subprocess_mock(INFO_COMMAND)
module.display_archives_info(
verbosity=None,
repository='repo',
storage_config={},
)
@ -26,21 +29,20 @@ def test_display_archives_info_calls_borg_with_parameters():
def test_display_archives_info_with_verbosity_some_calls_borg_with_info_parameter():
insert_subprocess_mock(INFO_COMMAND + ('--info',))
insert_logging_mock(logging.INFO)
module.display_archives_info(
repository='repo',
storage_config={},
verbosity=VERBOSITY_SOME,
)
def test_display_archives_info_with_verbosity_lots_calls_borg_with_debug_parameter():
insert_subprocess_mock(INFO_COMMAND + ('--debug',))
insert_subprocess_mock(INFO_COMMAND + ('--info', '--debug',))
insert_logging_mock(logging.DEBUG)
module.display_archives_info(
repository='repo',
storage_config={},
verbosity=VERBOSITY_LOTS,
)
@ -48,7 +50,6 @@ def test_display_archives_info_with_local_path_calls_borg_via_local_path():
insert_subprocess_mock(('borg1',) + INFO_COMMAND[1:])
module.display_archives_info(
verbosity=None,
repository='repo',
storage_config={},
local_path='borg1',
@ -59,7 +60,6 @@ def test_display_archives_info_with_remote_path_calls_borg_with_remote_path_para
insert_subprocess_mock(INFO_COMMAND + ('--remote-path', 'borg1'))
module.display_archives_info(
verbosity=None,
repository='repo',
storage_config={},
remote_path='borg1',
@ -71,7 +71,6 @@ def test_display_archives_info_with_lock_wait_calls_borg_with_lock_wait_paramete
insert_subprocess_mock(INFO_COMMAND + ('--lock-wait', '5'))
module.display_archives_info(
verbosity=None,
repository='repo',
storage_config=storage_config,
)

View File

@ -1,15 +1,20 @@
import logging
from collections import OrderedDict
from flexmock import flexmock
from borgmatic.borg import list as module
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
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 insert_logging_mock(log_level):
logging = flexmock(module.logging.Logger)
logging.should_receive('isEnabledFor').replace_with(lambda x : x >= log_level)
LIST_COMMAND = ('borg', 'list', 'repo')
@ -18,7 +23,6 @@ def test_list_archives_calls_borg_with_parameters():
insert_subprocess_mock(LIST_COMMAND)
module.list_archives(
verbosity=None,
repository='repo',
storage_config={},
)
@ -26,21 +30,21 @@ def test_list_archives_calls_borg_with_parameters():
def test_list_archives_with_verbosity_some_calls_borg_with_info_parameter():
insert_subprocess_mock(LIST_COMMAND + ('--info',))
insert_logging_mock(logging.INFO)
module.list_archives(
repository='repo',
storage_config={},
verbosity=VERBOSITY_SOME,
)
def test_list_archives_with_verbosity_lots_calls_borg_with_debug_parameter():
insert_subprocess_mock(LIST_COMMAND + ('--debug',))
insert_subprocess_mock(LIST_COMMAND + ('--info', '--debug',))
insert_logging_mock(logging.DEBUG)
module.list_archives(
repository='repo',
storage_config={},
verbosity=VERBOSITY_LOTS,
)
@ -48,7 +52,6 @@ def test_list_archives_with_local_path_calls_borg_via_local_path():
insert_subprocess_mock(('borg1',) + LIST_COMMAND[1:])
module.list_archives(
verbosity=None,
repository='repo',
storage_config={},
local_path='borg1',
@ -59,7 +62,6 @@ def test_list_archives_with_remote_path_calls_borg_with_remote_path_parameters()
insert_subprocess_mock(LIST_COMMAND + ('--remote-path', 'borg1'))
module.list_archives(
verbosity=None,
repository='repo',
storage_config={},
remote_path='borg1',
@ -71,7 +73,6 @@ def test_list_archives_with_lock_wait_calls_borg_with_lock_wait_parameters():
insert_subprocess_mock(LIST_COMMAND + ('--lock-wait', '5'))
module.list_archives(
verbosity=None,
repository='repo',
storage_config=storage_config,
)

View File

@ -1,9 +1,9 @@
import logging
from collections import OrderedDict
from flexmock import flexmock
from borgmatic.borg import prune as module
from borgmatic.verbosity import VERBOSITY_SOME, VERBOSITY_LOTS
def insert_subprocess_mock(check_call_command, **kwargs):
@ -11,6 +11,11 @@ def insert_subprocess_mock(check_call_command, **kwargs):
subprocess.should_receive('check_call').with_args(check_call_command, **kwargs).once()
def insert_logging_mock(log_level):
logging = flexmock(module.logging.Logger)
logging.should_receive('isEnabledFor').replace_with(lambda x : x >= log_level)
BASE_PRUNE_FLAGS = (
('--keep-daily', '1'),
('--keep-weekly', '2'),
@ -63,7 +68,6 @@ def test_prune_archives_calls_borg_with_parameters():
insert_subprocess_mock(PRUNE_COMMAND)
module.prune_archives(
verbosity=None,
dry_run=False,
repository='repo',
storage_config={},
@ -76,12 +80,12 @@ def test_prune_archives_with_verbosity_some_calls_borg_with_info_parameter():
flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return(
BASE_PRUNE_FLAGS,
)
insert_subprocess_mock(PRUNE_COMMAND + ('--info', '--stats',))
insert_subprocess_mock(PRUNE_COMMAND + ('--stats', '--info',))
insert_logging_mock(logging.INFO)
module.prune_archives(
repository='repo',
storage_config={},
verbosity=VERBOSITY_SOME,
dry_run=False,
retention_config=retention_config,
)
@ -92,12 +96,12 @@ def test_prune_archives_with_verbosity_lots_calls_borg_with_debug_parameter():
flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return(
BASE_PRUNE_FLAGS,
)
insert_subprocess_mock(PRUNE_COMMAND + ('--debug', '--stats', '--list'))
insert_subprocess_mock(PRUNE_COMMAND + ('--stats', '--info', '--debug', '--list'))
insert_logging_mock(logging.DEBUG)
module.prune_archives(
repository='repo',
storage_config={},
verbosity=VERBOSITY_LOTS,
dry_run=False,
retention_config=retention_config,
)
@ -113,7 +117,6 @@ def test_prune_archives_with_dry_run_calls_borg_with_dry_run_parameter():
module.prune_archives(
repository='repo',
storage_config={},
verbosity=None,
dry_run=True,
retention_config=retention_config,
)
@ -127,7 +130,6 @@ def test_prune_archives_with_local_path_calls_borg_via_local_path():
insert_subprocess_mock(('borg1',) + PRUNE_COMMAND[1:])
module.prune_archives(
verbosity=None,
dry_run=False,
repository='repo',
storage_config={},
@ -144,7 +146,6 @@ def test_prune_archives_with_remote_path_calls_borg_with_remote_path_parameters(
insert_subprocess_mock(PRUNE_COMMAND + ('--remote-path', 'borg1'))
module.prune_archives(
verbosity=None,
dry_run=False,
repository='repo',
storage_config={},
@ -162,7 +163,6 @@ def test_prune_archives_with_umask_calls_borg_with_umask_parameters():
insert_subprocess_mock(PRUNE_COMMAND + ('--umask', '077'))
module.prune_archives(
verbosity=None,
dry_run=False,
repository='repo',
storage_config=storage_config,
@ -179,7 +179,6 @@ def test_prune_archives_with_lock_wait_calls_borg_with_lock_wait_parameters():
insert_subprocess_mock(PRUNE_COMMAND + ('--lock-wait', '5'))
module.prune_archives(
verbosity=None,
dry_run=False,
repository='repo',
storage_config=storage_config,