Initial automated tests for delete action (#298).

borgmatic/actions/rdelete.py

@@ -20,7 +20,10 @@ def run_rdelete(
     if rdelete_arguments.repository is None or borgmatic.config.validate.repositories_match(
         repository, rdelete_arguments.repository
     ):
-        logger.answer(f'{repository.get("label", repository["path"])}: Deleting repository' + (' cache' if rdelete_arguments.cache_only else ''))
+        logger.answer(
+            f'{repository.get("label", repository["path"])}: Deleting repository'
+            + (' cache' if rdelete_arguments.cache_only else '')
+        )
 
         borgmatic.borg.rdelete.delete_repository(
             repository,

borgmatic/borg/delete.py

@@ -84,7 +84,8 @@ def delete_archives(
     borgmatic.logger.add_custom_log_levels()
 
     if not any(
-        getattr(delete_arguments, argument_name) for argument_name in ARCHIVE_RELATED_ARGUMENT_NAMES
+        getattr(delete_arguments, argument_name, None)
+        for argument_name in ARCHIVE_RELATED_ARGUMENT_NAMES
     ):
         if borgmatic.borg.feature.available(
             borgmatic.borg.feature.Feature.RDELETE, local_borg_version
@@ -100,7 +101,7 @@ def delete_archives(
             cache_only=delete_arguments.cache_only,
             keep_security_info=delete_arguments.keep_security_info,
         )
-        return borgmatic.borg.rdelete.delete_repository(
+        borgmatic.borg.rdelete.delete_repository(
             repository,
             config,
             local_borg_version,
@@ -110,6 +111,8 @@ def delete_archives(
             remote_path,
         )
 
+        return
+
     command = make_delete_command(
         repository,
         config,

docs/how-to/develop-on-borgmatic.md

@@ -155,8 +155,8 @@ the following deviations from it:
  * Favor blank lines around logical code groupings, `if` statements,
    `return`s, etc. Readability is more important than packing code tightly.
  * Import fully qualified Python modules instead of importing individual
-   functions, classes, or constants (e.g., do `import os.path` instead of
-   `from os import path`).
+   functions, classes, or constants. E.g., do `import os.path` instead of
+   `from os import path`. (Some exceptions to this are made in tests.)
 
 borgmatic code uses the [Black](https://black.readthedocs.io/en/stable/) code
 formatter, the [Flake8](http://flake8.pycqa.org/en/latest/) code checker, and

tests/unit/actions/test_delete.py

@@ -0,0 +1,43 @@
+from flexmock import flexmock
+
+from borgmatic.actions import delete as module
+
+
+def test_run_delete_does_not_raise():
+    flexmock(module.logger).answer = lambda message: None
+    flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True)
+    flexmock(module.borgmatic.borg.rlist).should_receive('resolve_archive_name')
+    flexmock(module.borgmatic.actions.arguments).should_receive('update_arguments').and_return(
+        flexmock()
+    )
+    flexmock(module.borgmatic.borg.delete).should_receive('delete_archives')
+
+    module.run_delete(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version=None,
+        delete_arguments=flexmock(repository=flexmock(), archive=flexmock()),
+        global_arguments=flexmock(),
+        local_path=None,
+        remote_path=None,
+    )
+
+
+def test_run_delete_without_archive_does_not_raise():
+    flexmock(module.logger).answer = lambda message: None
+    flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True)
+    flexmock(module.borgmatic.borg.rlist).should_receive('resolve_archive_name')
+    flexmock(module.borgmatic.actions.arguments).should_receive('update_arguments').and_return(
+        flexmock()
+    )
+    flexmock(module.borgmatic.borg.delete).should_receive('delete_archives')
+
+    module.run_delete(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version=None,
+        delete_arguments=flexmock(repository=flexmock(), archive=None),
+        global_arguments=flexmock(),
+        local_path=None,
+        remote_path=None,
+    )

tests/unit/actions/test_rdelete.py

@@ -0,0 +1,41 @@
+from flexmock import flexmock
+
+from borgmatic.actions import rdelete as module
+
+
+def test_run_rdelete_does_not_raise():
+    flexmock(module.logger).answer = lambda message: None
+    flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True)
+    flexmock(module.borgmatic.actions.arguments).should_receive('update_arguments').and_return(
+        flexmock()
+    )
+    flexmock(module.borgmatic.borg.rdelete).should_receive('delete_repository')
+
+    module.run_rdelete(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version=None,
+        rdelete_arguments=flexmock(repository=flexmock(), cache_only=False),
+        global_arguments=flexmock(),
+        local_path=None,
+        remote_path=None,
+    )
+
+
+def test_run_rdelete_with_cache_only_does_not_raise():
+    flexmock(module.logger).answer = lambda message: None
+    flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True)
+    flexmock(module.borgmatic.actions.arguments).should_receive('update_arguments').and_return(
+        flexmock()
+    )
+    flexmock(module.borgmatic.borg.rdelete).should_receive('delete_repository')
+
+    module.run_rdelete(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version=None,
+        rdelete_arguments=flexmock(repository=flexmock(), cache_only=True),
+        global_arguments=flexmock(),
+        local_path=None,
+        remote_path=None,
+    )

tests/unit/borg/test_delete.py

@@ -0,0 +1,338 @@
+import logging
+
+import pytest
+from flexmock import flexmock
+
+from borgmatic.borg import delete as module
+
+from ..test_verbosity import insert_logging_mock
+
+
+def test_make_delete_command_includes_log_info():
+    insert_logging_mock(logging.INFO)
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(list_archives=False, force=0, match_archives=None, archive=None),
+        global_arguments=flexmock(dry_run=False, log_json=False),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--info', 'repo')
+
+
+def test_make_delete_command_includes_log_debug():
+    insert_logging_mock(logging.DEBUG)
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(list_archives=False, force=0, match_archives=None, archive=None),
+        global_arguments=flexmock(dry_run=False, log_json=False),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--debug', '--show-rc', 'repo')
+
+
+def test_make_delete_command_includes_dry_run():
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').with_args(
+        'dry-run', True
+    ).and_return(('--dry-run',))
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(list_archives=False, force=0, match_archives=None, archive=None),
+        global_arguments=flexmock(dry_run=True, log_json=False),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--dry-run', 'repo')
+
+
+def test_make_delete_command_includes_remote_path():
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').with_args(
+        'remote-path', 'borg1'
+    ).and_return(('--remote-path', 'borg1'))
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(list_archives=False, force=0, match_archives=None, archive=None),
+        global_arguments=flexmock(dry_run=False, log_json=False),
+        local_path='borg',
+        remote_path='borg1',
+    )
+
+    assert command == ('borg', 'delete', '--remote-path', 'borg1', 'repo')
+
+
+def test_make_delete_command_includes_log_json():
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').with_args(
+        'log-json', True
+    ).and_return(('--log-json',))
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(list_archives=False, force=0, match_archives=None, archive=None),
+        global_arguments=flexmock(dry_run=False, log_json=True),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--log-json', 'repo')
+
+
+def test_make_delete_command_includes_lock_wait():
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').with_args(
+        'lock-wait', 5
+    ).and_return(('--lock-wait', '5'))
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={'lock_wait': 5},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(list_archives=False, force=0, match_archives=None, archive=None),
+        global_arguments=flexmock(dry_run=False, log_json=False),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--lock-wait', '5', 'repo')
+
+
+def test_make_delete_command_includes_list():
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').with_args(
+        'list', True
+    ).and_return(('--list',))
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(list_archives=True, force=0, match_archives=None, archive=None),
+        global_arguments=flexmock(dry_run=False, log_json=False),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--list', 'repo')
+
+
+def test_make_delete_command_includes_force():
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(list_archives=False, force=1, match_archives=None, archive=None),
+        global_arguments=flexmock(dry_run=False, log_json=False),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--force', 'repo')
+
+
+def test_make_delete_command_includes_force_twice():
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(list_archives=False, force=2, match_archives=None, archive=None),
+        global_arguments=flexmock(dry_run=False, log_json=False),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--force', '--force', 'repo')
+
+
+def test_make_delete_command_includes_archive():
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(
+        ('--match-archives', 'archive')
+    )
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(
+            list_archives=False, force=0, match_archives=None, archive='archive'
+        ),
+        global_arguments=flexmock(dry_run=False, log_json=False),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--match-archives', 'archive', 'repo')
+
+
+def test_make_delete_command_includes_match_archives():
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_match_archives_flags').and_return(
+        ('--match-archives', 'sh:foo*')
+    )
+    flexmock(module.borgmatic.borg.flags).should_receive('make_flags_from_arguments').and_return(())
+    flexmock(module.borgmatic.borg.flags).should_receive('make_repository_flags').and_return(
+        ('repo',)
+    )
+
+    command = module.make_delete_command(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version='1.2.3',
+        delete_arguments=flexmock(
+            list_archives=False, force=0, match_archives='sh:foo*', archive='archive'
+        ),
+        global_arguments=flexmock(dry_run=False, log_json=False),
+        local_path='borg',
+        remote_path=None,
+    )
+
+    assert command == ('borg', 'delete', '--match-archives', 'sh:foo*', 'repo')
+
+
+def test_delete_archives_with_archive_calls_borg_delete():
+    flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels')
+    flexmock(module.borgmatic.borg.rdelete).should_receive('delete_repository').never()
+    flexmock(module).should_receive('make_delete_command').and_return(flexmock())
+    flexmock(module.borgmatic.borg.environment).should_receive('make_environment').and_return(
+        flexmock()
+    )
+    flexmock(module.borgmatic.execute).should_receive('execute_command').once()
+
+    module.delete_archives(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version=flexmock(),
+        delete_arguments=flexmock(archive='archive'),
+        global_arguments=flexmock(),
+    )
+
+
+def test_delete_archives_with_match_archives_calls_borg_delete():
+    flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels')
+    flexmock(module.borgmatic.borg.rdelete).should_receive('delete_repository').never()
+    flexmock(module).should_receive('make_delete_command').and_return(flexmock())
+    flexmock(module.borgmatic.borg.environment).should_receive('make_environment').and_return(
+        flexmock()
+    )
+    flexmock(module.borgmatic.execute).should_receive('execute_command').once()
+
+    module.delete_archives(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version=flexmock(),
+        delete_arguments=flexmock(match_archives='sh:foo*'),
+        global_arguments=flexmock(),
+    )
+
+
+@pytest.mark.parametrize('argument_name', module.ARCHIVE_RELATED_ARGUMENT_NAMES[2:])
+def test_delete_archives_with_archive_related_argument_calls_borg_delete(argument_name):
+    flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels')
+    flexmock(module.borgmatic.borg.rdelete).should_receive('delete_repository').never()
+    flexmock(module).should_receive('make_delete_command').and_return(flexmock())
+    flexmock(module.borgmatic.borg.environment).should_receive('make_environment').and_return(
+        flexmock()
+    )
+    flexmock(module.borgmatic.execute).should_receive('execute_command').once()
+
+    module.delete_archives(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version=flexmock(),
+        delete_arguments=flexmock(archive='archive', **{argument_name: 'value'}),
+        global_arguments=flexmock(),
+    )
+
+
+def test_delete_archives_without_archive_related_argument_calls_borg_rdelete():
+    flexmock(module.borgmatic.logger).should_receive('add_custom_log_levels')
+    flexmock(module.borgmatic.borg.feature).should_receive('available').and_return(True)
+    flexmock(module.borgmatic.borg.rdelete).should_receive('delete_repository').once()
+    flexmock(module).should_receive('make_delete_command').never()
+    flexmock(module.borgmatic.borg.environment).should_receive('make_environment').never()
+    flexmock(module.borgmatic.execute).should_receive('execute_command').never()
+
+    module.delete_archives(
+        repository={'path': 'repo'},
+        config={},
+        local_borg_version=flexmock(),
+        delete_arguments=flexmock(
+            list_archives=True, force=False, cache_only=False, keep_security_info=False
+        ),
+        global_arguments=flexmock(),
+    )