fix label handling and improve unittests

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,)
         }

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)