Add some missing test coverage (#80).

borgmatic/hooks/data_source/btrfs.py

@@ -146,7 +146,7 @@ def make_snapshot_exclude_path(subvolume_path):  # pragma: no cover
     )
 
 
-def make_borg_source_directory_path(subvolume_path, source_directory):
+def make_borg_source_directory_path(subvolume_path, source_directory):  # pragma: no cover
     '''
     Given the path to a subvolume and a source directory inside it, make a corresponding path for
     the source directory within a snapshot path intended for giving to Borg.

borgmatic/hooks/data_source/lvm.py

@@ -330,9 +330,9 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
             if not dry_run:
                 shutil.rmtree(snapshot_mount_path, ignore_errors=True)
 
-            # If the delete was successful, that means there's nothing to unmount.
-            if not os.path.isdir(snapshot_mount_path):
-                continue
+                # If the delete was successful, that means there's nothing to unmount.
+                if not os.path.isdir(snapshot_mount_path):
+                    continue
 
             logger.debug(
                 f'{log_prefix}: Unmounting LVM snapshot at {snapshot_mount_path}{dry_run_label}'

borgmatic/hooks/data_source/zfs.py

@@ -332,9 +332,9 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
             if not dry_run:
                 shutil.rmtree(snapshot_mount_path, ignore_errors=True)
 
-            # If the delete was successful, that means there's nothing to unmount.
-            if not os.path.isdir(snapshot_mount_path):
-                continue
+                # If the delete was successful, that means there's nothing to unmount.
+                if not os.path.isdir(snapshot_mount_path):
+                    continue
 
             logger.debug(
                 f'{log_prefix}: Unmounting ZFS snapshot at {snapshot_mount_path}{dry_run_label}'

tests/unit/hooks/data_source/test_snapshot.py

@@ -0,0 +1,26 @@
+from borgmatic.hooks.data_source import snapshot as module
+
+
+def test_get_contained_directories_without_candidates_returns_empty():
+    assert module.get_contained_directories('/mnt', {}) == ()
+
+
+def test_get_contained_directories_with_self_candidate_returns_self():
+    candidates = {'/foo', '/mnt', '/bar'}
+
+    assert module.get_contained_directories('/mnt', candidates) == ('/mnt',)
+    assert candidates == {'/foo', '/bar'}
+
+
+def test_get_contained_directories_with_child_candidate_returns_child():
+    candidates = {'/foo', '/mnt/subdir', '/bar'}
+
+    assert module.get_contained_directories('/mnt', candidates) == ('/mnt/subdir',)
+    assert candidates == {'/foo', '/bar'}
+
+
+def test_get_contained_directories_with_grandchild_candidate_returns_child():
+    candidates = {'/foo', '/mnt/sub/dir', '/bar'}
+
+    assert module.get_contained_directories('/mnt', candidates) == ('/mnt/sub/dir',)
+    assert candidates == {'/foo', '/bar'}

tests/unit/hooks/data_source/test_zfs.py

@@ -216,6 +216,46 @@ def test_dump_data_sources_with_dry_run_skips_commands_and_does_not_touch_source
     assert source_directories == ['/mnt/dataset']
 
 
+def test_dump_data_sources_ignores_mismatch_between_source_directories_and_contained_source_directories():
+    flexmock(module).should_receive('get_datasets_to_backup').and_return(
+        (
+            flexmock(
+                name='dataset',
+                mount_point='/mnt/dataset',
+                contained_source_directories=('/mnt/dataset/subdir',),
+            )
+        )
+    )
+    flexmock(module.os).should_receive('getpid').and_return(1234)
+    full_snapshot_name = 'dataset@borgmatic-1234'
+    flexmock(module).should_receive('snapshot_dataset').with_args(
+        'zfs',
+        full_snapshot_name,
+    ).once()
+    snapshot_mount_path = '/run/borgmatic/zfs_snapshots/./mnt/dataset'
+    flexmock(module).should_receive('mount_snapshot').with_args(
+        'mount',
+        full_snapshot_name,
+        module.os.path.normpath(snapshot_mount_path),
+    ).once()
+    source_directories = ['/hmm']
+
+    assert (
+        module.dump_data_sources(
+            hook_config={},
+            config={'source_directories': '/mnt/dataset', 'zfs': {}},
+            log_prefix='test',
+            config_paths=('test.yaml',),
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=False,
+        )
+        == []
+    )
+
+    assert source_directories == ['/hmm', os.path.join(snapshot_mount_path, 'subdir')]
+
+
 def test_get_all_snapshots_parses_list_output():
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output'
@@ -418,6 +458,36 @@ def test_remove_data_source_dumps_skips_unmount_snapshot_mount_paths_that_are_no
     )
 
 
+def test_remove_data_source_dumps_skips_unmount_snapshot_mount_paths_after_rmtree_succeeds():
+    flexmock(module).should_receive('get_all_dataset_mount_points').and_return(('/mnt/dataset',))
+    flexmock(module.borgmatic.config.paths).should_receive(
+        'replace_temporary_subdirectory_with_glob'
+    ).and_return('/run/borgmatic')
+    flexmock(module.glob).should_receive('glob').replace_with(lambda path: [path])
+    flexmock(module.os.path).should_receive('isdir').with_args(
+        '/run/borgmatic/zfs_snapshots'
+    ).and_return(True)
+    flexmock(module.os.path).should_receive('isdir').with_args(
+        '/run/borgmatic/zfs_snapshots/mnt/dataset'
+    ).and_return(True).and_return(False)
+    flexmock(module.shutil).should_receive('rmtree')
+    flexmock(module).should_receive('unmount_snapshot').never()
+    flexmock(module).should_receive('get_all_snapshots').and_return(
+        ('dataset@borgmatic-1234', 'dataset@other', 'other@other', 'invalid')
+    )
+    flexmock(module).should_receive('destroy_snapshot').with_args(
+        'zfs', 'dataset@borgmatic-1234'
+    ).once()
+
+    module.remove_data_source_dumps(
+        hook_config={},
+        config={'source_directories': '/mnt/dataset', 'zfs': {}},
+        log_prefix='test',
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=False,
+    )
+
+
 def test_remove_data_source_dumps_with_dry_run_skips_unmount_and_destroy():
     flexmock(module).should_receive('get_all_dataset_mount_points').and_return(('/mnt/dataset',))
     flexmock(module.borgmatic.config.paths).should_receive(