WIP Replace subprocess calls with a generic run function #109

Closed
floli wants to merge 2 commits from (deleted):run_function into main
14 changed files with 126 additions and 123 deletions

View File

@ -3,6 +3,7 @@ import os
import subprocess
from borgmatic.borg import extract
from borgmatic.run import run
DEFAULT_CHECKS = ('repository', 'archives')
@ -117,8 +118,7 @@ def check_archives(
# The check command spews to stdout/stderr even without the verbose flag. Suppress it.
stdout = None if verbosity_flags else open(os.devnull, 'w')
logger.debug(' '.join(full_command))
subprocess.check_call(full_command, stdout=stdout, stderr=subprocess.STDOUT)
run(full_command, stdout=stdout, stderr=subprocess.STDOUT)
if 'extract' in checks:
extract.extract_last_archive_dry_run(repository, lock_wait, local_path, remote_path)

View File

@ -2,9 +2,9 @@ import glob
import itertools
import logging
import os
import subprocess
import tempfile
from borgmatic.run import run
logger = logging.getLogger(__name__)
@ -154,5 +154,4 @@ def create_archive(
+ (('--json',) if json else ())
)
logger.debug(' '.join(full_command))
subprocess.check_call(full_command)
return run(full_command, capture_output = json)
Review

Note that when you run the Black code formatter on this (part of running borgmatic's tox), it will complain about the format on lines like this one. You can always run tox -e black though to have it auto-reformat them for you.

Note that when you run the [Black](https://github.com/ambv/black) code formatter on this (part of running borgmatic's tox), it will complain about the format on lines like this one. You can always run `tox -e black` though to have it auto-reformat them for you.

View File

@ -1,7 +1,7 @@
import logging
import sys
import subprocess
from borgmatic.run import run
logger = logging.getLogger(__name__)
@ -26,7 +26,7 @@ def extract_last_archive_dry_run(repository, lock_wait=None, local_path='borg',
+ verbosity_flags
)
list_output = subprocess.check_output(full_list_command).decode(sys.stdout.encoding)
list_output = run(full_list_command, capture_output = True)
last_archive_name = list_output.strip().split('\n')[-1]
if not last_archive_name:
@ -48,5 +48,4 @@ def extract_last_archive_dry_run(repository, lock_wait=None, local_path='borg',
+ list_flag
)
logger.debug(' '.join(full_extract_command))
subprocess.check_call(full_extract_command)
run(full_extract_command)

View File

@ -1,6 +1,6 @@
import logging
import subprocess
from borgmatic.run import run
logger = logging.getLogger(__name__)
@ -23,7 +23,4 @@ def display_archives_info(
+ (('--json',) if json else ())
)
logger.debug(' '.join(full_command))
output = subprocess.check_output(full_command)
return output.decode() if output is not None else None
return run(full_command, capture_output = json)

View File

@ -1,6 +1,6 @@
import logging
import subprocess
from borgmatic.run import run
logger = logging.getLogger(__name__)
@ -20,7 +20,5 @@ def list_archives(repository, storage_config, local_path='borg', remote_path=Non
+ (('--debug', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ())
+ (('--json',) if json else ())
)
logger.debug(' '.join(full_command))
output = subprocess.check_output(full_command)
return output.decode() if output is not None else None
run(full_command, capture_output = json)
Review

Needs to return the result of the run() invocation?

Needs to `return` the result of the `run()` invocation?

View File

@ -1,6 +1,6 @@
import logging
import subprocess
from borgmatic.run import run
logger = logging.getLogger(__name__)
@ -53,5 +53,4 @@ def prune_archives(
+ (('--dry-run',) if dry_run else ())
)
logger.debug(' '.join(full_command))
subprocess.check_call(full_command)
run(full_command)

View File

@ -205,7 +205,7 @@ def _run_commands_on_repository(
)
if args.create:
logger.info('{}: Creating archive{}'.format(repository, dry_run_label))
borg_create.create_archive(
output = borg_create.create_archive(
args.dry_run,
repository,
location,
@ -213,6 +213,8 @@ def _run_commands_on_repository(
local_path=local_path,
remote_path=remote_path,
)
if args.json:
json_results.append(json.loads(output))
Review

Would it make sense to do if output: instead of if args.json here? That way, you don't have the args.json-checking logic in two places (1. in create_archive() and, 2. here). Similar elsewhere in this function. Do no feel super strongly.

Would it make sense to do `if output:` instead of `if args.json` here? That way, you don't have the `args.json`-checking logic in two places (1. in `create_archive()` and, 2. here). Similar elsewhere in this function. Do no feel super strongly.
if args.check and checks.repository_enabled_for_checks(repository, consistency):
logger.info('{}: Running consistency checks'.format(repository))
borg_check.check_archives(
@ -225,8 +227,6 @@ def _run_commands_on_repository(
)
if args.json:
json_results.append(json.loads(output))
else:
sys.stdout.write(output)
if args.info:
logger.info('{}: Displaying summary info for archives'.format(repository))
output = borg_info.display_archives_info(
@ -234,8 +234,6 @@ def _run_commands_on_repository(
)
if args.json:
json_results.append(json.loads(output))
else:
sys.stdout.write(output)
def main(): # pragma: no cover

14
borgmatic/run.py Normal file
View File

@ -0,0 +1,14 @@
import logging
import subprocess
logger = logging.getLogger(__name__)
def run(args, capture_output = False, **kwargs):
Review

Would be nice to have a doc string for this function, even though it's pretty self-explanitory.

Also, given that this function is only used within borgmatic/borg/, this file might belong in that directory. (Do not feel strongly.)

Would be nice to have a doc string for this function, even though it's pretty self-explanitory. Also, given that this function is only used within `borgmatic/borg/`, this file might belong in that directory. (Do not feel strongly.)
logger.debug('Executing: ' + ' '.join(args))
Review

Might be a little more idiomatic to do something like: logger.debug('Executing: {}'.format(...))

Might be a little more idiomatic to do something like: `logger.debug('Executing: {}'.format(...))`
if capture_output:
return subprocess.check_output(args, **kwargs).decode()
subprocess.check_call(args, **kwargs)
Review

This function could use some very basic test coverage. Basically: The capture output case, and the no capture output case. That'd require a little bit of mocking.

This function could use some very basic test coverage. Basically: The capture output case, and the no capture output case. That'd require a little bit of mocking.

View File

@ -8,15 +8,13 @@ import pytest
from borgmatic.borg import check as module
from ..test_verbosity import insert_logging_mock
def insert_run_mock(command, capture_output = False, **kwargs):
mocked_module = flexmock(module)
mocked_module.should_receive('run').with_args(command, capture_output, **kwargs).once()
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_subprocess_never():
subprocess = flexmock(module.subprocess)
subprocess.should_receive('check_call').never()
def insert_run_never():
mocked_module = flexmock(module)
mocked_module.should_receive('run').never()
def test_parse_checks_returns_them_as_tuple():
@ -120,7 +118,7 @@ def test_check_archives_calls_borg_with_parameters(checks):
checks, check_last, None
).and_return(())
stdout = flexmock()
insert_subprocess_mock(('borg', 'check', 'repo'), stdout=stdout, stderr=STDOUT)
insert_run_mock(('borg', 'check', 'repo'), stdout=stdout, stderr=STDOUT)
flexmock(sys.modules['builtins']).should_receive('open').and_return(stdout)
flexmock(module.os).should_receive('devnull')
@ -136,7 +134,7 @@ def test_check_archives_with_extract_check_calls_extract_only():
flexmock(module).should_receive('_parse_checks').and_return(checks)
flexmock(module).should_receive('_make_check_flags').never()
flexmock(module.extract).should_receive('extract_last_archive_dry_run').once()
insert_subprocess_never()
insert_run_never()
module.check_archives(
repository='repo', storage_config={}, consistency_config=consistency_config
@ -149,7 +147,7 @@ def test_check_archives_with_log_info_calls_borg_with_info_parameter():
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)
insert_run_mock(('borg', 'check', 'repo', '--info'), stdout=None, stderr=STDOUT)
module.check_archives(
repository='repo', storage_config={}, consistency_config=consistency_config
@ -162,7 +160,7 @@ def test_check_archives_with_log_debug_calls_borg_with_debug_parameter():
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(
insert_run_mock(
('borg', 'check', 'repo', '--debug', '--show-rc'), stdout=None, stderr=STDOUT
)
@ -174,7 +172,7 @@ def test_check_archives_with_log_debug_calls_borg_with_debug_parameter():
def test_check_archives_without_any_checks_bails():
consistency_config = {'check_last': None}
flexmock(module).should_receive('_parse_checks').and_return(())
insert_subprocess_never()
insert_run_never()
module.check_archives(
repository='repo', storage_config={}, consistency_config=consistency_config
@ -190,7 +188,7 @@ def test_check_archives_with_local_path_calls_borg_via_local_path():
checks, check_last, None
).and_return(())
stdout = flexmock()
insert_subprocess_mock(('borg1', 'check', 'repo'), stdout=stdout, stderr=STDOUT)
insert_run_mock(('borg1', 'check', 'repo'), stdout=stdout, stderr=STDOUT)
flexmock(sys.modules['builtins']).should_receive('open').and_return(stdout)
flexmock(module.os).should_receive('devnull')
@ -211,7 +209,7 @@ def test_check_archives_with_remote_path_calls_borg_with_remote_path_parameters(
checks, check_last, None
).and_return(())
stdout = flexmock()
insert_subprocess_mock(
insert_run_mock(
('borg', 'check', 'repo', '--remote-path', 'borg1'), stdout=stdout, stderr=STDOUT
)
flexmock(sys.modules['builtins']).should_receive('open').and_return(stdout)
@ -234,7 +232,7 @@ def test_check_archives_with_lock_wait_calls_borg_with_lock_wait_parameters():
checks, check_last, None
).and_return(())
stdout = flexmock()
insert_subprocess_mock(
insert_run_mock(
('borg', 'check', 'repo', '--lock-wait', '5'), stdout=stdout, stderr=STDOUT
)
flexmock(sys.modules['builtins']).should_receive('open').and_return(stdout)
@ -255,7 +253,7 @@ def test_check_archives_with_retention_prefix():
checks, check_last, prefix
).and_return(())
stdout = flexmock()
insert_subprocess_mock(('borg', 'check', 'repo'), stdout=stdout, stderr=STDOUT)
insert_run_mock(('borg', 'check', 'repo'), stdout=stdout, stderr=STDOUT)
flexmock(sys.modules['builtins']).should_receive('open').and_return(stdout)
flexmock(module.os).should_receive('devnull')

View File

@ -6,6 +6,10 @@ from flexmock import flexmock
from borgmatic.borg import create as module
from ..test_verbosity import insert_logging_mock
def insert_run_mock(command, capture_output = False, **kwargs):
mocked_module = flexmock(module)
mocked_module.should_receive('run').with_args(command, capture_output, **kwargs).once()
def test_initialize_environment_with_passcommand_should_set_environment():
orig_environ = os.environ
@ -200,7 +204,7 @@ def test_create_archive_calls_borg_with_parameters():
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND)
insert_run_mock(CREATE_COMMAND)
module.create_archive(
dry_run=False,
@ -222,7 +226,7 @@ def test_create_archive_with_patterns_calls_borg_with_patterns():
).and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(pattern_flags)
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + pattern_flags)
insert_run_mock(CREATE_COMMAND + pattern_flags)
module.create_archive(
dry_run=False,
@ -246,7 +250,7 @@ def test_create_archive_with_exclude_patterns_calls_borg_with_excludes():
)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(exclude_flags)
insert_subprocess_mock(CREATE_COMMAND + exclude_flags)
insert_run_mock(CREATE_COMMAND + exclude_flags)
module.create_archive(
dry_run=False,
@ -266,7 +270,7 @@ def test_create_archive_with_log_info_calls_borg_with_info_parameter():
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--list', '--filter', 'AME', '--info', '--stats'))
insert_run_mock(CREATE_COMMAND + ('--list', '--filter', 'AME', '--info', '--stats'))
insert_logging_mock(logging.INFO)
module.create_archive(
@ -286,7 +290,7 @@ def test_create_archive_with_log_debug_calls_borg_with_debug_parameter():
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(
insert_run_mock(
CREATE_COMMAND + ('--list', '--filter', 'AME', '--stats', '--debug', '--show-rc')
)
insert_logging_mock(logging.DEBUG)
@ -309,7 +313,7 @@ def test_create_archive_with_dry_run_calls_borg_with_dry_run_parameter():
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--dry-run',))
insert_run_mock(CREATE_COMMAND + ('--dry-run',))
module.create_archive(
dry_run=True,
@ -331,7 +335,7 @@ def test_create_archive_with_dry_run_and_log_info_calls_borg_without_stats_param
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--list', '--filter', 'AME', '--info', '--dry-run'))
insert_run_mock(CREATE_COMMAND + ('--list', '--filter', 'AME', '--info', '--dry-run'))
insert_logging_mock(logging.INFO)
module.create_archive(
@ -354,7 +358,7 @@ def test_create_archive_with_dry_run_and_log_debug_calls_borg_without_stats_para
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(
insert_run_mock(
CREATE_COMMAND + ('--list', '--filter', 'AME', '--debug', '--show-rc', '--dry-run')
)
insert_logging_mock(logging.DEBUG)
@ -376,7 +380,7 @@ def test_create_archive_with_checkpoint_interval_calls_borg_with_checkpoint_inte
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--checkpoint-interval', '600'))
insert_run_mock(CREATE_COMMAND + ('--checkpoint-interval', '600'))
module.create_archive(
dry_run=False,
@ -395,7 +399,7 @@ def test_create_archive_with_chunker_params_calls_borg_with_chunker_params_param
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--chunker-params', '1,2,3,4'))
insert_run_mock(CREATE_COMMAND + ('--chunker-params', '1,2,3,4'))
module.create_archive(
dry_run=False,
@ -414,7 +418,7 @@ def test_create_archive_with_compression_calls_borg_with_compression_parameters(
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--compression', 'rle'))
insert_run_mock(CREATE_COMMAND + ('--compression', 'rle'))
module.create_archive(
dry_run=False,
@ -433,7 +437,7 @@ def test_create_archive_with_remote_rate_limit_calls_borg_with_remote_ratelimit_
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--remote-ratelimit', '100'))
insert_run_mock(CREATE_COMMAND + ('--remote-ratelimit', '100'))
module.create_archive(
dry_run=False,
@ -452,7 +456,7 @@ def test_create_archive_with_one_file_system_calls_borg_with_one_file_system_par
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--one-file-system',))
insert_run_mock(CREATE_COMMAND + ('--one-file-system',))
module.create_archive(
dry_run=False,
@ -472,7 +476,7 @@ def test_create_archive_with_read_special_calls_borg_with_read_special_parameter
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--read-special',))
insert_run_mock(CREATE_COMMAND + ('--read-special',))
module.create_archive(
dry_run=False,
@ -492,7 +496,7 @@ def test_create_archive_with_bsd_flags_true_calls_borg_without_nobsdflags_parame
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND)
insert_run_mock(CREATE_COMMAND)
module.create_archive(
dry_run=False,
@ -512,7 +516,7 @@ def test_create_archive_with_bsd_flags_false_calls_borg_with_nobsdflags_paramete
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--nobsdflags',))
insert_run_mock(CREATE_COMMAND + ('--nobsdflags',))
module.create_archive(
dry_run=False,
@ -532,7 +536,7 @@ def test_create_archive_with_files_cache_calls_borg_with_files_cache_parameters(
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--files-cache', 'ctime,size'))
insert_run_mock(CREATE_COMMAND + ('--files-cache', 'ctime,size'))
module.create_archive(
dry_run=False,
@ -552,7 +556,7 @@ def test_create_archive_with_local_path_calls_borg_via_local_path():
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(('borg1',) + CREATE_COMMAND[1:])
insert_run_mock(('borg1',) + CREATE_COMMAND[1:])
module.create_archive(
dry_run=False,
@ -572,7 +576,7 @@ def test_create_archive_with_remote_path_calls_borg_with_remote_path_parameters(
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--remote-path', 'borg1'))
insert_run_mock(CREATE_COMMAND + ('--remote-path', 'borg1'))
module.create_archive(
dry_run=False,
@ -592,7 +596,7 @@ def test_create_archive_with_umask_calls_borg_with_umask_parameters():
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--umask', '740'))
insert_run_mock(CREATE_COMMAND + ('--umask', '740'))
module.create_archive(
dry_run=False,
@ -611,7 +615,7 @@ def test_create_archive_with_lock_wait_calls_borg_with_lock_wait_parameters():
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--lock-wait', '5'))
insert_run_mock(CREATE_COMMAND + ('--lock-wait', '5'))
module.create_archive(
dry_run=False,
@ -630,7 +634,7 @@ def test_create_archive_with_json_calls_borg_with_json_parameter():
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(CREATE_COMMAND + ('--json',))
insert_run_mock(CREATE_COMMAND + ('--json',), capture_output = True)
module.create_archive(
dry_run=False,
@ -652,7 +656,7 @@ def test_create_archive_with_source_directories_glob_expands():
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(
insert_run_mock(
('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food')
)
flexmock(module.glob).should_receive('glob').with_args('foo*').and_return(['foo', 'food'])
@ -674,7 +678,7 @@ def test_create_archive_with_non_matching_source_directories_glob_passes_through
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo*'))
insert_run_mock(('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo*'))
flexmock(module.glob).should_receive('glob').with_args('foo*').and_return([])
module.create_archive(
@ -696,7 +700,7 @@ def test_create_archive_with_glob_calls_borg_with_expanded_directories():
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(
insert_run_mock(
('borg', 'create', 'repo::{}'.format(DEFAULT_ARCHIVE_NAME), 'foo', 'food')
)
@ -717,7 +721,7 @@ def test_create_archive_with_archive_name_format_calls_borg_with_archive_name():
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(('borg', 'create', 'repo::ARCHIVE_NAME', 'foo', 'bar'))
insert_run_mock(('borg', 'create', 'repo::ARCHIVE_NAME', 'foo', 'bar'))
module.create_archive(
dry_run=False,
@ -736,7 +740,7 @@ def test_create_archive_with_archive_name_format_accepts_borg_placeholders():
flexmock(module).should_receive('_write_pattern_file').and_return(None)
flexmock(module).should_receive('_make_pattern_flags').and_return(())
flexmock(module).should_receive('_make_exclude_flags').and_return(())
insert_subprocess_mock(('borg', 'create', 'repo::Documents_{hostname}-{now}', 'foo', 'bar'))
insert_run_mock(('borg', 'create', 'repo::Documents_{hostname}-{now}', 'foo', 'bar'))
module.create_archive(
dry_run=False,

View File

@ -7,29 +7,26 @@ from borgmatic.borg import extract as module
from ..test_verbosity import insert_logging_mock
def insert_subprocess_mock(check_call_command, **kwargs):
subprocess = flexmock(module.subprocess)
subprocess.should_receive('check_call').with_args(check_call_command, **kwargs).once()
def insert_run_mock(command, capture_output = False, **kwargs):
Review

I think capture_output isn't used here, and so should be removed.

I think `capture_output` isn't used here, and so should be removed.
mocked_module = flexmock(module)
mocked_module.should_receive('run').with_args(command, capture_output, **kwargs).once()
def insert_run_never():
mocked_module = flexmock(module)
mocked_module.should_receive('run').never()
def insert_subprocess_never():
subprocess = flexmock(module.subprocess)
subprocess.should_receive('check_call').never()
def insert_subprocess_check_output_mock(check_output_command, result, **kwargs):
subprocess = flexmock(module.subprocess)
subprocess.should_receive('check_output').with_args(check_output_command, **kwargs).and_return(
result
).once()
def insert_run_output_mock(command, result, **kwargs):
mocked_module = flexmock(module)
mocked_module.should_receive('run').with_args(command, capture_output = True, **kwargs).and_return(result).once()
def test_extract_last_archive_dry_run_should_call_borg_with_last_archive():
flexmock(sys.stdout).encoding = 'utf-8'
insert_subprocess_check_output_mock(
insert_run_output_mock(
('borg', 'list', '--short', 'repo'), result='archive1\narchive2\n'.encode('utf-8')
)
insert_subprocess_mock(('borg', 'extract', '--dry-run', 'repo::archive2'))
insert_run_mock(('borg', 'extract', '--dry-run', 'repo::archive2'))
module.extract_last_archive_dry_run(repository='repo', lock_wait=None)
@ -39,7 +36,7 @@ def test_extract_last_archive_dry_run_without_any_archives_should_bail():
insert_subprocess_check_output_mock(
('borg', 'list', '--short', 'repo'), result='\n'.encode('utf-8')
)
insert_subprocess_never()
insert_run_never()
module.extract_last_archive_dry_run(repository='repo', lock_wait=None)
@ -49,7 +46,7 @@ def test_extract_last_archive_dry_run_with_log_info_should_call_borg_with_info_p
insert_subprocess_check_output_mock(
('borg', 'list', '--short', 'repo', '--info'), result='archive1\narchive2\n'.encode('utf-8')
)
insert_subprocess_mock(('borg', 'extract', '--dry-run', 'repo::archive2', '--info'))
insert_run_mock(('borg', 'extract', '--dry-run', 'repo::archive2', '--info'))
insert_logging_mock(logging.INFO)
module.extract_last_archive_dry_run(repository='repo', lock_wait=None)
@ -61,7 +58,7 @@ def test_extract_last_archive_dry_run_with_log_debug_should_call_borg_with_debug
('borg', 'list', '--short', 'repo', '--debug', '--show-rc'),
result='archive1\narchive2\n'.encode('utf-8'),
)
insert_subprocess_mock(
insert_run_mock(
('borg', 'extract', '--dry-run', 'repo::archive2', '--debug', '--show-rc', '--list')
)
insert_logging_mock(logging.DEBUG)
@ -74,7 +71,7 @@ def test_extract_last_archive_dry_run_should_call_borg_via_local_path():
insert_subprocess_check_output_mock(
('borg1', 'list', '--short', 'repo'), result='archive1\narchive2\n'.encode('utf-8')
)
insert_subprocess_mock(('borg1', 'extract', '--dry-run', 'repo::archive2'))
insert_run_mock(('borg1', 'extract', '--dry-run', 'repo::archive2'))
module.extract_last_archive_dry_run(repository='repo', lock_wait=None, local_path='borg1')
@ -85,7 +82,7 @@ def test_extract_last_archive_dry_run_should_call_borg_with_remote_path_paramete
('borg', 'list', '--short', 'repo', '--remote-path', 'borg1'),
result='archive1\narchive2\n'.encode('utf-8'),
)
insert_subprocess_mock(
insert_run_mock(
('borg', 'extract', '--dry-run', 'repo::archive2', '--remote-path', 'borg1')
)
@ -94,10 +91,10 @@ def test_extract_last_archive_dry_run_should_call_borg_with_remote_path_paramete
def test_extract_last_archive_dry_run_should_call_borg_with_lock_wait_parameters():
flexmock(sys.stdout).encoding = 'utf-8'
insert_subprocess_check_output_mock(
insert_run_output_mock(
('borg', 'list', '--short', 'repo', '--lock-wait', '5'),
result='archive1\narchive2\n'.encode('utf-8'),
Review

I don't think you want this .encode() here. The unit under test (extract_last_archive_dry_run()) is expecting an already decoded result string returned from run(). Or in this case, the mock run().

I don't think you want this `.encode()` here. The unit under test (`extract_last_archive_dry_run()`) is expecting an already decoded result string returned from `run()`. Or in this case, the mock `run()`.
)
insert_subprocess_mock(('borg', 'extract', '--dry-run', 'repo::archive2', '--lock-wait', '5'))
insert_run_mock(('borg', 'extract', '--dry-run', 'repo::archive2', '--lock-wait', '5'))
module.extract_last_archive_dry_run(repository='repo', lock_wait=5)
Review

So I looked a bit into the causes of these tests failing. It looks like insert_run_output_mock() and insert_run_mock() both create mocks for run(), which interfere with one another. This wasn't an issue previously with insert_subprocess_check_output_mock() and insert_subprocess_mock(), because each one created a mock for a different underlying subprocess function.

Here's what I would recommend: Remove the .once() from the mock creation within each insert test function. That'll prevent the two mocks from interfering with one another. It does make the assertion a little weaker. (It's not actually asserting that it's called.) But that seems fine.

Also, I think you'll need to change insert_run_mock() not to expect capture_output for this to work.

So I looked a bit into the causes of these tests failing. It looks like `insert_run_output_mock()` and `insert_run_mock()` both create mocks for `run()`, which interfere with one another. This wasn't an issue previously with `insert_subprocess_check_output_mock()` and `insert_subprocess_mock()`, because each one created a mock for a different underlying `subprocess` function. Here's what I would recommend: Remove the `.once()` from the mock creation within each insert test function. That'll prevent the two mocks from interfering with one another. It does make the assertion a little weaker. (It's not actually asserting that it's called.) But that seems fine. Also, I think you'll need to change `insert_run_mock()` not to expect `capture_output` for this to work.

View File

@ -6,53 +6,53 @@ from borgmatic.borg import info as module
from ..test_verbosity import insert_logging_mock
def insert_subprocess_mock(check_call_command, **kwargs):
subprocess = flexmock(module.subprocess)
subprocess.should_receive('check_output').with_args(check_call_command, **kwargs).once()
def insert_run_mock(command, capture_output = False, **kwargs):
mocked_module = flexmock(module)
mocked_module.should_receive('run').with_args(command, capture_output, **kwargs).once()
INFO_COMMAND = ('borg', 'info', 'repo')
def test_display_archives_info_calls_borg_with_parameters():
insert_subprocess_mock(INFO_COMMAND)
insert_run_mock(INFO_COMMAND)
module.display_archives_info(repository='repo', storage_config={})
def test_display_archives_info_with_log_info_calls_borg_with_info_parameter():
insert_subprocess_mock(INFO_COMMAND + ('--info',))
insert_run_mock(INFO_COMMAND + ('--info',))
insert_logging_mock(logging.INFO)
module.display_archives_info(repository='repo', storage_config={})
def test_display_archives_info_with_log_debug_calls_borg_with_debug_parameter():
insert_subprocess_mock(INFO_COMMAND + ('--debug', '--show-rc'))
insert_run_mock(INFO_COMMAND + ('--debug', '--show-rc'))
insert_logging_mock(logging.DEBUG)
module.display_archives_info(repository='repo', storage_config={})
def test_display_archives_info_with_json_calls_borg_with_json_parameter():
insert_subprocess_mock(INFO_COMMAND + ('--json',))
insert_run_mock(INFO_COMMAND + ('--json',), capture_output = True)
module.display_archives_info(repository='repo', storage_config={}, json=True)
def test_display_archives_info_with_local_path_calls_borg_via_local_path():
insert_subprocess_mock(('borg1',) + INFO_COMMAND[1:])
insert_run_mock(('borg1',) + INFO_COMMAND[1:])
module.display_archives_info(repository='repo', storage_config={}, local_path='borg1')
def test_display_archives_info_with_remote_path_calls_borg_with_remote_path_parameters():
insert_subprocess_mock(INFO_COMMAND + ('--remote-path', 'borg1'))
insert_run_mock(INFO_COMMAND + ('--remote-path', 'borg1'))
module.display_archives_info(repository='repo', storage_config={}, remote_path='borg1')
def test_display_archives_info_with_lock_wait_calls_borg_with_lock_wait_parameters():
storage_config = {'lock_wait': 5}
insert_subprocess_mock(INFO_COMMAND + ('--lock-wait', '5'))
insert_run_mock(INFO_COMMAND + ('--lock-wait', '5'))
module.display_archives_info(repository='repo', storage_config=storage_config)

View File

@ -6,54 +6,54 @@ from borgmatic.borg import list as module
from ..test_verbosity import insert_logging_mock
def insert_subprocess_mock(check_call_command, **kwargs):
subprocess = flexmock(module.subprocess)
subprocess.should_receive('check_output').with_args(check_call_command, **kwargs).once()
def insert_run_mock(command, capture_output = False, **kwargs):
mocked_module = flexmock(module)
mocked_module.should_receive('run').with_args(command, capture_output, **kwargs).once()
LIST_COMMAND = ('borg', 'list', 'repo')
def test_list_archives_calls_borg_with_parameters():
insert_subprocess_mock(LIST_COMMAND)
insert_run_mock(LIST_COMMAND)
module.list_archives(repository='repo', storage_config={})
def test_list_archives_with_log_info_calls_borg_with_info_parameter():
insert_subprocess_mock(LIST_COMMAND + ('--info',))
insert_run_mock(LIST_COMMAND + ('--info',))
insert_logging_mock(logging.INFO)
module.list_archives(repository='repo', storage_config={})
def test_list_archives_with_log_debug_calls_borg_with_debug_parameter():
insert_subprocess_mock(LIST_COMMAND + ('--debug', '--show-rc'))
insert_run_mock(LIST_COMMAND + ('--debug', '--show-rc'))
insert_logging_mock(logging.DEBUG)
module.list_archives(repository='repo', storage_config={})
def test_list_archives_with_json_calls_borg_with_json_parameter():
insert_subprocess_mock(LIST_COMMAND + ('--json',))
insert_run_mock(LIST_COMMAND + ('--json',), capture_output = True)
module.list_archives(repository='repo', storage_config={}, json=True)
def test_list_archives_with_local_path_calls_borg_via_local_path():
insert_subprocess_mock(('borg1',) + LIST_COMMAND[1:])
insert_run_mock(('borg1',) + LIST_COMMAND[1:])
module.list_archives(repository='repo', storage_config={}, local_path='borg1')
def test_list_archives_with_remote_path_calls_borg_with_remote_path_parameters():
insert_subprocess_mock(LIST_COMMAND + ('--remote-path', 'borg1'))
insert_run_mock(LIST_COMMAND + ('--remote-path', 'borg1'))
module.list_archives(repository='repo', storage_config={}, remote_path='borg1')
def test_list_archives_with_lock_wait_calls_borg_with_lock_wait_parameters():
storage_config = {'lock_wait': 5}
insert_subprocess_mock(LIST_COMMAND + ('--lock-wait', '5'))
insert_run_mock(LIST_COMMAND + ('--lock-wait', '5'))
module.list_archives(repository='repo', storage_config=storage_config)

View File

@ -7,9 +7,9 @@ from borgmatic.borg import prune as module
from ..test_verbosity import insert_logging_mock
def insert_subprocess_mock(check_call_command, **kwargs):
subprocess = flexmock(module.subprocess)
subprocess.should_receive('check_call').with_args(check_call_command, **kwargs).once()
def insert_run_mock(command, **kwargs):
mocked_module = flexmock(module)
mocked_module.should_receive('run').with_args(command, **kwargs).once()
BASE_PRUNE_FLAGS = (('--keep-daily', '1'), ('--keep-weekly', '2'), ('--keep-monthly', '3'))
@ -51,7 +51,7 @@ def test_prune_archives_calls_borg_with_parameters():
flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return(
BASE_PRUNE_FLAGS
)
insert_subprocess_mock(PRUNE_COMMAND)
insert_run_mock(PRUNE_COMMAND)
module.prune_archives(
dry_run=False, repository='repo', storage_config={}, retention_config=retention_config
@ -63,7 +63,7 @@ def test_prune_archives_with_log_info_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 + ('--stats', '--info'))
insert_run_mock(PRUNE_COMMAND + ('--stats', '--info'))
insert_logging_mock(logging.INFO)
module.prune_archives(
@ -76,7 +76,7 @@ def test_prune_archives_with_log_debug_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 + ('--stats', '--debug', '--list', '--show-rc'))
insert_run_mock(PRUNE_COMMAND + ('--stats', '--debug', '--list', '--show-rc'))
insert_logging_mock(logging.DEBUG)
module.prune_archives(
@ -89,7 +89,7 @@ def test_prune_archives_with_dry_run_calls_borg_with_dry_run_parameter():
flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return(
BASE_PRUNE_FLAGS
)
insert_subprocess_mock(PRUNE_COMMAND + ('--dry-run',))
insert_run_mock(PRUNE_COMMAND + ('--dry-run',))
module.prune_archives(
repository='repo', storage_config={}, dry_run=True, retention_config=retention_config
@ -101,7 +101,7 @@ def test_prune_archives_with_local_path_calls_borg_via_local_path():
flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return(
BASE_PRUNE_FLAGS
)
insert_subprocess_mock(('borg1',) + PRUNE_COMMAND[1:])
insert_run_mock(('borg1',) + PRUNE_COMMAND[1:])
module.prune_archives(
dry_run=False,
@ -117,7 +117,7 @@ def test_prune_archives_with_remote_path_calls_borg_with_remote_path_parameters(
flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return(
BASE_PRUNE_FLAGS
)
insert_subprocess_mock(PRUNE_COMMAND + ('--remote-path', 'borg1'))
insert_run_mock(PRUNE_COMMAND + ('--remote-path', 'borg1'))
module.prune_archives(
dry_run=False,
@ -134,7 +134,7 @@ def test_prune_archives_with_umask_calls_borg_with_umask_parameters():
flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return(
BASE_PRUNE_FLAGS
)
insert_subprocess_mock(PRUNE_COMMAND + ('--umask', '077'))
insert_run_mock(PRUNE_COMMAND + ('--umask', '077'))
module.prune_archives(
dry_run=False,
@ -150,7 +150,7 @@ def test_prune_archives_with_lock_wait_calls_borg_with_lock_wait_parameters():
flexmock(module).should_receive('_make_prune_flags').with_args(retention_config).and_return(
BASE_PRUNE_FLAGS
)
insert_subprocess_mock(PRUNE_COMMAND + ('--lock-wait', '5'))
insert_run_mock(PRUNE_COMMAND + ('--lock-wait', '5'))
module.prune_archives(
dry_run=False,