From 8639b73f8070946c645f86d6e589d3ebedd19364 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Thu, 31 Jul 2025 09:23:54 +0200 Subject: [PATCH] fix label handling and improve unittests --- borgmatic/actions/restore.py | 8 ++-- tests/unit/actions/test_restore.py | 67 ++++++++++++++++++++++-------- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index c325454b..21ce7e5d 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -35,9 +35,11 @@ def dumps_match(first, second, default_port=None): ''' # label kinda counts as id, if they match ignore hostname & port if first.label not in {None, UNSPECIFIED} and first.label == second.label: - return first[:2] == second[:2] + field_list = ('hook_name', 'data_source_name') + else: + field_list = Dump._fields - for field_name in first._fields: + for field_name in field_list: first_value = getattr(first, field_name) second_value = getattr(second, field_name) @@ -422,7 +424,7 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): data_source_name=name, hostname=restore_arguments.original_hostname or UNSPECIFIED, port=restore_arguments.original_port, - label=restore_arguments.original_label, + label=restore_arguments.original_label or UNSPECIFIED, ) for name in restore_arguments.data_sources or (UNSPECIFIED,) } diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index ec8488b0..f1b5d891 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -132,6 +132,12 @@ import borgmatic.actions.restore as module 5432, True, ), + ( + module.Dump('postgresql_databases', 'foo', 'some_host1', 5433, 'unique'), + module.Dump(module.UNSPECIFIED, 'foo', 'some_host2', None, 'unique'), + 5432, + True, + ), ), ) def test_dumps_match_compares_two_dumps_while_respecting_unspecified_values( @@ -730,11 +736,15 @@ def test_get_dumps_to_restore_gets_requested_dumps_found_in_archive(): } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), + module.Dump( + module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED, label=module.UNSPECIFIED + ), module.Dump('postgresql_databases', 'foo'), ).and_return(True) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'bar', hostname=module.UNSPECIFIED), + module.Dump( + module.UNSPECIFIED, 'bar', hostname=module.UNSPECIFIED, label=module.UNSPECIFIED + ), module.Dump('postgresql_databases', 'bar'), ).and_return(True) @@ -758,9 +768,15 @@ def test_get_dumps_to_restore_raises_for_requested_dumps_missing_from_archive(): module.Dump('postgresql_databases', 'foo'), } flexmock(module).should_receive('dumps_match').and_return(False) - flexmock(module).should_receive('render_dump_metadata').and_return('test') + flexmock(module).should_receive('dumps_match').with_args( + module.Dump( + module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED, label=module.UNSPECIFIED + ), + module.Dump('postgresql_databases', 'foo'), + ).and_return(True) + flexmock(module).should_receive('render_dump_metadata').and_return('test').once() - with pytest.raises(ValueError): + with pytest.raises(ValueError) as exc_info: module.get_dumps_to_restore( restore_arguments=flexmock( hook=None, @@ -771,6 +787,7 @@ def test_get_dumps_to_restore_raises_for_requested_dumps_missing_from_archive(): ), dumps_from_archive=dumps_from_archive, ) + assert 'dump test missing from archive' in str(exc_info.value) def test_get_dumps_to_restore_without_requested_dumps_finds_all_archive_dumps(): @@ -832,11 +849,15 @@ def test_get_dumps_to_restore_with_all_in_requested_dumps_plus_additional_reques } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), + module.Dump( + module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED, label=module.UNSPECIFIED + ), module.Dump('postgresql_databases', 'foo'), ).and_return(True) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'bar', hostname=module.UNSPECIFIED), + module.Dump( + module.UNSPECIFIED, 'bar', hostname=module.UNSPECIFIED, label=module.UNSPECIFIED + ), module.Dump('postgresql_databases', 'bar'), ).and_return(True) @@ -858,16 +879,20 @@ def test_get_dumps_to_restore_with_all_in_requested_dumps_plus_additional_reques def test_get_dumps_to_restore_raises_for_multiple_matching_dumps_in_archive(): flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), + module.Dump( + module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED, label=module.UNSPECIFIED + ), module.Dump('postgresql_databases', 'foo'), ).and_return(True) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), + module.Dump( + module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED, label=module.UNSPECIFIED + ), module.Dump('mariadb_databases', 'foo'), ).and_return(True) flexmock(module).should_receive('render_dump_metadata').and_return('test') - with pytest.raises(ValueError): + with pytest.raises(ValueError) as exc_info: module.get_dumps_to_restore( restore_arguments=flexmock( hook=None, @@ -881,17 +906,20 @@ def test_get_dumps_to_restore_raises_for_multiple_matching_dumps_in_archive(): module.Dump('mariadb_databases', 'foo'), }, ) + assert 'Try adding flags to disambiguate.' in str(exc_info.value) def test_get_dumps_to_restore_raises_for_all_in_requested_dumps_and_requested_dumps_missing_from_archive(): flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump(module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED), + module.Dump( + module.UNSPECIFIED, 'foo', hostname=module.UNSPECIFIED, label=module.UNSPECIFIED + ), module.Dump('postgresql_databases', 'foo'), ).and_return(True) - flexmock(module).should_receive('render_dump_metadata').and_return('test') + flexmock(module).should_receive('render_dump_metadata').and_return('test').once() - with pytest.raises(ValueError): + with pytest.raises(ValueError) as exc_info: module.get_dumps_to_restore( restore_arguments=flexmock( hook=None, @@ -900,8 +928,9 @@ def test_get_dumps_to_restore_raises_for_all_in_requested_dumps_and_requested_du original_port=None, original_label=None, ), - dumps_from_archive={module.Dump('postresql_databases', 'foo')}, + dumps_from_archive={module.Dump('postgresql_databases', 'foo')}, ) + assert 'dump test missing from archive' in str(exc_info.value) def test_get_dumps_to_restore_with_requested_hook_name_filters_dumps_found_in_archive(): @@ -912,7 +941,9 @@ def test_get_dumps_to_restore_with_requested_hook_name_filters_dumps_found_in_ar } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump('postgresql_databases', 'foo', hostname=module.UNSPECIFIED), + module.Dump( + 'postgresql_databases', 'foo', hostname=module.UNSPECIFIED, label=module.UNSPECIFIED + ), module.Dump('postgresql_databases', 'foo'), ).and_return(True) @@ -938,7 +969,9 @@ def test_get_dumps_to_restore_with_requested_shortened_hook_name_filters_dumps_f } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump('postgresql_databases', 'foo', hostname=module.UNSPECIFIED), + module.Dump( + 'postgresql_databases', 'foo', hostname=module.UNSPECIFIED, label=module.UNSPECIFIED + ), module.Dump('postgresql_databases', 'foo'), ).and_return(True) @@ -964,7 +997,7 @@ def test_get_dumps_to_restore_with_requested_hostname_filters_dumps_found_in_arc } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump('postgresql_databases', 'foo', 'host'), + module.Dump('postgresql_databases', 'foo', 'host', label=module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo', 'host'), ).and_return(True) @@ -990,7 +1023,7 @@ def test_get_dumps_to_restore_with_requested_port_filters_dumps_found_in_archive } flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( - module.Dump('postgresql_databases', 'foo', 'host', 1234), + module.Dump('postgresql_databases', 'foo', 'host', 1234, label=module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo', 'host', 1234), ).and_return(True)