From d978a2d19053bf029345504f8cce63820774c359 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 25 Nov 2019 15:27:59 -0800 Subject: [PATCH] Fix for database dump removal incorrectly skipping some database dumps. --- NEWS | 1 + borgmatic/hooks/dump.py | 6 +++--- tests/unit/hooks/test_dump.py | 20 ++++++++++++-------- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index 1a5db8d1..7e411212 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,5 @@ 1.4.15 + * Fix for database dump removal incorrectly skipping some database dumps. * #123: Support for mounting an archive as a FUSE filesystem via "borgmatic mount" action, and unmounting via "borgmatic umount". See the documentation for more information: https://torsion.org/borgmatic/docs/how-to/extract-a-backup/#mount-a-filesystem diff --git a/borgmatic/hooks/dump.py b/borgmatic/hooks/dump.py index d0c32e93..38905efd 100644 --- a/borgmatic/hooks/dump.py +++ b/borgmatic/hooks/dump.py @@ -71,10 +71,10 @@ def remove_database_dumps(dump_path, databases, database_type_name, log_prefix, continue os.remove(dump_filename) - dump_path = os.path.dirname(dump_filename) + dump_file_dir = os.path.dirname(dump_filename) - if len(os.listdir(dump_path)) == 0: - os.rmdir(dump_path) + if len(os.listdir(dump_file_dir)) == 0: + os.rmdir(dump_file_dir) def convert_glob_patterns_to_borg_patterns(patterns): diff --git a/tests/unit/hooks/test_dump.py b/tests/unit/hooks/test_dump.py index 93d54e64..b74d5b7a 100644 --- a/tests/unit/hooks/test_dump.py +++ b/tests/unit/hooks/test_dump.py @@ -51,16 +51,20 @@ def test_flatten_dump_patterns_with_no_hooks_errors(): def test_remove_database_dumps_removes_dump_for_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] - flexmock(module).should_receive('make_database_dump_filename').and_return( - 'databases/localhost/foo' + flexmock(module).should_receive('make_database_dump_filename').with_args( + 'databases', 'foo', None + ).and_return('databases/localhost/foo') + flexmock(module).should_receive('make_database_dump_filename').with_args( + 'databases', 'bar', None ).and_return('databases/localhost/bar') - flexmock(module.os).should_receive('listdir').and_return([]) - flexmock(module.os).should_receive('rmdir') - for name in ('foo', 'bar'): - flexmock(module.os).should_receive('remove').with_args( - 'databases/localhost/{}'.format(name) - ).once() + flexmock(module.os).should_receive('remove').with_args('databases/localhost/foo').once() + flexmock(module.os).should_receive('remove').with_args('databases/localhost/bar').once() + flexmock(module.os).should_receive('listdir').with_args('databases/localhost').and_return( + ['bar'] + ).and_return([]) + + flexmock(module.os).should_receive('rmdir').with_args('databases/localhost').once() module.remove_database_dumps('databases', databases, 'SuperDB', 'test.yaml', dry_run=False)