Add test coverage for ZFS hook (#261).

README.md

@@ -61,6 +61,7 @@ borgmatic is powered by [Borg Backup](https://www.borgbackup.org/).
 <a href="https://mariadb.com/"><img src="docs/static/mariadb.png" alt="MariaDB" height="60px" style="margin-bottom:20px; margin-right:20px;"></a>
 <a href="https://www.mongodb.com/"><img src="docs/static/mongodb.png" alt="MongoDB" height="60px" style="margin-bottom:20px; margin-right:20px;"></a>
 <a href="https://sqlite.org/"><img src="docs/static/sqlite.png" alt="SQLite" height="60px" style="margin-bottom:20px; margin-right:20px;"></a>
+<a href="https://openzfs.org/"><img src="docs/static/openzfs.png" alt="OpenZFS" height="60px" style="margin-bottom:20px; margin-right:20px;"></a>
 <a href="https://healthchecks.io/"><img src="docs/static/healthchecks.png" alt="Healthchecks" height="60px" style="margin-bottom:20px; margin-right:20px;"></a>
 <a href="https://uptime.kuma.pet/"><img src="docs/static/uptimekuma.png" alt="Uptime Kuma" height="60px" style="margin-bottom:20px; margin-right:20px;"></a>
 <a href="https://cronitor.io/"><img src="docs/static/cronitor.png" alt="Cronitor" height="60px" style="margin-bottom:20px; margin-right:20px;"></a>

borgmatic/hooks/zfs.py

@@ -10,7 +10,7 @@ import borgmatic.execute
 logger = logging.getLogger(__name__)
 
 
-def use_streaming(hook_config, config, log_prefix):
+def use_streaming(hook_config, config, log_prefix):  # pragma: no cover
     '''
     Return whether dump streaming is used for this hook. (Spoiler: It isn't.)
     '''
@@ -26,7 +26,7 @@ def get_datasets_to_backup(zfs_command, source_directories):
     Given a ZFS command to run and a sequence of configured source directories, find the
     intersection between the current ZFS dataset mount points and the configured borgmatic source
     directories. The idea is that these are the requested datasets to snapshot. But also include any
-    datasets tagged with a borgmatic-specific user property whether or not they appear in source
+    datasets tagged with a borgmatic-specific user property, whether or not they appear in source
     directories.
 
     Return the result as a sequence of (dataset name, mount point) pairs.
@@ -44,12 +44,15 @@ def get_datasets_to_backup(zfs_command, source_directories):
     )
     source_directories_set = set(source_directories)
 
-    return tuple(
-        (dataset_name, mount_point)
-        for line in list_output.splitlines()
-        for (dataset_name, mount_point, user_property_value) in (line.rstrip().split('\t'),)
-        if mount_point in source_directories_set or user_property_value == 'auto'
-    )
+    try:
+        return tuple(
+            (dataset_name, mount_point)
+            for line in list_output.splitlines()
+            for (dataset_name, mount_point, user_property_value) in (line.rstrip().split('\t'),)
+            if mount_point in source_directories_set or user_property_value == 'auto'
+        )
+    except ValueError:
+        raise ValueError('Invalid {zfs_command} list output')
 
 
 def get_all_datasets(zfs_command):
@@ -69,14 +72,17 @@ def get_all_datasets(zfs_command):
         )
     )
 
-    return tuple(
-        (dataset_name, mount_point)
-        for line in list_output.splitlines()
-        for (dataset_name, mount_point) in (line.rstrip().split('\t'),)
-    )
+    try:
+        return tuple(
+            (dataset_name, mount_point)
+            for line in list_output.splitlines()
+            for (dataset_name, mount_point) in (line.rstrip().split('\t'),)
+        )
+    except ValueError:
+        raise ValueError('Invalid {zfs_command} list output')
 
 
-def snapshot_dataset(zfs_command, full_snapshot_name):
+def snapshot_dataset(zfs_command, full_snapshot_name):  # pragma: no cover
     '''
     Given a ZFS command to run and a snapshot name of the form "dataset@snapshot", create a new ZFS
     snapshot.
@@ -92,7 +98,7 @@ def snapshot_dataset(zfs_command, full_snapshot_name):
     )
 
 
-def mount_snapshot(mount_command, full_snapshot_name, snapshot_mount_path):
+def mount_snapshot(mount_command, full_snapshot_name, snapshot_mount_path):  # pragma: no cover
     '''
     Given a mount command to run, an existing snapshot name of the form "dataset@snapshot", and the
     path where the snapshot should be mounted, mount the snapshot (making any necessary directories
@@ -122,12 +128,12 @@ def dump_data_sources(
     '''
     Given a ZFS configuration dict, a configuration dict, a log prefix, the borgmatic runtime
     directory, the configured source directories, and whether this is a dry run, auto-detect and
-    snapshot any ZFS dataset mount points listed in the given source directories and also any
-    dataset with a borgmatic-specific user property. Also update those source directories, replacing
-    dataset mount points with corresponding snapshot directories. Use the log prefix in any log
-    entries.
+    snapshot any ZFS dataset mount points listed in the given source directories and any dataset
+    with a borgmatic-specific user property. Also update those source directories, replacing dataset
+    mount points with corresponding snapshot directories so they get stored in the Borg archive
+    instead of the dataset mount points. Use the log prefix in any log entries.
 
-    Return an empty sequence, since there are no ongoing dump processes.
+    Return an empty sequence, since there are no ongoing dump processes from this hook.
 
     If this is a dry run, then don't actually snapshot anything.
     '''
@@ -174,7 +180,7 @@ def dump_data_sources(
     return []
 
 
-def unmount_snapshot(umount_command, snapshot_mount_path):
+def unmount_snapshot(umount_command, snapshot_mount_path):  # pragma: no cover
     '''
     Given a umount command to run and the mount path of a snapshot, unmount it.
     '''
@@ -187,7 +193,7 @@ def unmount_snapshot(umount_command, snapshot_mount_path):
     )
 
 
-def destroy_snapshot(zfs_command, full_snapshot_name):
+def destroy_snapshot(zfs_command, full_snapshot_name):  # pragma: no cover
     '''
     Given a ZFS command to run and the name of a snapshot in the form "dataset@snapshot", destroy
     it.
@@ -246,7 +252,7 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
 
     snapshots_glob = os.path.join(
         borgmatic.config.paths.replace_temporary_subdirectory_with_glob(
-            os.path.normpath(borgmatic_runtime_directory)
+            os.path.normpath(borgmatic_runtime_directory),
         ),
         'zfs_snapshots',
     )
@@ -263,7 +269,8 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
         # we'll try again below. The point of doing it here is that we don't want to try to unmount
         # a non-mounted directory (which *will* fail), and probing for whether a directory is
         # mounted is tough to do in a cross-platform way.
-        shutil.rmtree(snapshots_directory, ignore_errors=True)
+        if not dry_run:
+            shutil.rmtree(snapshots_directory, ignore_errors=True)
 
         for _, mount_point in datasets:
             snapshot_mount_path = os.path.join(snapshots_directory, mount_point.lstrip(os.path.sep))
@@ -277,7 +284,8 @@ def remove_data_source_dumps(hook_config, config, log_prefix, borgmatic_runtime_
             if not dry_run:
                 unmount_snapshot(umount_command, snapshot_mount_path)
 
-        shutil.rmtree(snapshots_directory)
+        if not dry_run:
+            shutil.rmtree(snapshots_directory)
 
     # Destroy snapshots.
     full_snapshot_names = get_all_snapshots(zfs_command)

pyproject.toml

@@ -49,7 +49,7 @@ skip-string-normalization = true
 
 [tool.pytest.ini_options]
 testpaths = "tests"
-addopts = "--cov-report term-missing:skip-covered --cov=borgmatic --ignore=tests/end-to-end"
+addopts = "--cov-report term-missing:skip-covered --cov=borgmatic --no-cov-on-fail --cov-fail-under=100 --ignore=tests/end-to-end"
 
 [tool.isort]
 profile = "black"

tests/unit/actions/test_create.py

@@ -207,13 +207,45 @@ def test_pattern_root_directories_parses_roots_and_ignores_others():
     ) == ['/root', '/baz']
 
 
-# TODO
-# def test_process_source_directories_...
-#    flexmock(module).should_receive('deduplicate_directories').and_return(('foo', 'bar'))
-#    flexmock(module).should_receive('map_directories_to_devices').and_return({})
-#    flexmock(module).should_receive('expand_directories').and_return(())
-#    flexmock(module).should_receive('pattern_root_directories').and_return([])
-#    ...
+def test_process_source_directories_includes_source_directories_and_config_paths():
+    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(
+        '/working'
+    )
+    flexmock(module).should_receive('deduplicate_directories').and_return(
+        ('foo', 'bar', 'test.yaml')
+    )
+    flexmock(module).should_receive('map_directories_to_devices').and_return({})
+    flexmock(module).should_receive('expand_directories').with_args(
+        ('foo', 'bar', 'test.yaml'), working_directory='/working'
+    ).and_return(()).once()
+    flexmock(module).should_receive('pattern_root_directories').and_return(())
+    flexmock(module).should_receive('expand_directories').with_args(
+        (), working_directory='/working'
+    ).and_return(())
+
+    assert module.process_source_directories(
+        config={'source_directories': ['foo', 'bar']}, config_paths=('test.yaml',)
+    ) == ('foo', 'bar', 'test.yaml')
+
+
+def test_process_source_directories_does_not_include_config_paths_when_store_config_files_is_false():
+    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(
+        '/working'
+    )
+    flexmock(module).should_receive('deduplicate_directories').and_return(('foo', 'bar'))
+    flexmock(module).should_receive('map_directories_to_devices').and_return({})
+    flexmock(module).should_receive('expand_directories').with_args(
+        ('foo', 'bar'), working_directory='/working'
+    ).and_return(()).once()
+    flexmock(module).should_receive('pattern_root_directories').and_return(())
+    flexmock(module).should_receive('expand_directories').with_args(
+        (), working_directory='/working'
+    ).and_return(())
+
+    assert module.process_source_directories(
+        config={'source_directories': ['foo', 'bar'], 'store_config_files': False},
+        config_paths=('test.yaml',),
+    ) == ('foo', 'bar')
 
 
 def test_run_create_executes_and_calls_hooks_for_configured_repository():

tests/unit/config/test_paths.py

@@ -22,6 +22,10 @@ def test_expand_user_in_path_handles_none_directory():
     assert module.expand_user_in_path(None) is None
 
 
+def test_expand_user_in_path_handles_incorrectly_typed_directory():
+    assert module.expand_user_in_path(3) is None
+
+
 def test_get_borgmatic_source_directory_uses_config_option():
     flexmock(module).should_receive('expand_user_in_path').replace_with(lambda path: path)
 
@@ -34,6 +38,13 @@ def test_get_borgmatic_source_directory_without_config_option_uses_default():
     assert module.get_borgmatic_source_directory({}) == '~/.borgmatic'
 
 
+def test_replace_temporary_subdirectory_with_glob_transforms_path():
+    assert (
+        module.replace_temporary_subdirectory_with_glob('/tmp/borgmatic-aet8kn93/borgmatic')
+        == '/tmp/borgmatic-*/borgmatic'
+    )
+
+
 def test_runtime_directory_uses_config_option():
     flexmock(module).should_receive('expand_user_in_path').replace_with(lambda path: path)
     flexmock(module.os).should_receive('makedirs')
@@ -154,6 +165,25 @@ def test_runtime_directory_falls_back_to_hard_coded_tmp_path_and_adds_temporary_
         assert borgmatic_runtime_directory == '/tmp/borgmatic-1234/./borgmatic'
 
 
+def test_runtime_directory_with_erroring_cleanup_does_not_raise():
+    flexmock(module).should_receive('expand_user_in_path').replace_with(lambda path: path)
+    flexmock(module.os.environ).should_receive('get').with_args('XDG_RUNTIME_DIR').and_return(None)
+    flexmock(module.os.environ).should_receive('get').with_args('RUNTIME_DIRECTORY').and_return(
+        None
+    )
+    flexmock(module.os.environ).should_receive('get').with_args('TMPDIR').and_return(None)
+    flexmock(module.os.environ).should_receive('get').with_args('TEMP').and_return(None)
+    temporary_directory = flexmock(name='/tmp/borgmatic-1234')
+    temporary_directory.should_receive('cleanup').and_raise(OSError).once()
+    flexmock(module.tempfile).should_receive('TemporaryDirectory').with_args(
+        prefix='borgmatic-', dir='/tmp'
+    ).and_return(temporary_directory)
+    flexmock(module.os).should_receive('makedirs')
+
+    with module.Runtime_directory({}, 'prefix') as borgmatic_runtime_directory:
+        assert borgmatic_runtime_directory == '/tmp/borgmatic-1234/./borgmatic'
+
+
 @pytest.mark.parametrize(
     'borgmatic_runtime_directory,expected_glob',
     (

tests/unit/hooks/test_zfs.py

@@ -0,0 +1,326 @@
+import pytest
+from flexmock import flexmock
+
+import borgmatic.execute
+from borgmatic.hooks import zfs as module
+
+
+def test_get_datasets_to_backup_filters_datasets_by_source_directories():
+    flexmock(borgmatic.execute).should_receive('execute_command_and_capture_output').and_return(
+        'dataset\t/dataset\t-\nother\t/other\t-',
+    )
+
+    assert module.get_datasets_to_backup(
+        'zfs', source_directories=('/foo', '/dataset', '/bar')
+    ) == (('dataset', '/dataset'),)
+
+
+def test_get_datasets_to_backup_filters_datasets_by_user_property():
+    flexmock(borgmatic.execute).should_receive('execute_command_and_capture_output').and_return(
+        'dataset\t/dataset\tauto\nother\t/other\t-',
+    )
+
+    assert module.get_datasets_to_backup('zfs', source_directories=('/foo', '/bar')) == (
+        ('dataset', '/dataset'),
+    )
+
+
+def test_get_datasets_to_backup_with_invalid_list_output_raises():
+    flexmock(borgmatic.execute).should_receive('execute_command_and_capture_output').and_return(
+        'dataset',
+    )
+
+    with pytest.raises(ValueError, match='zfs'):
+        module.get_datasets_to_backup('zfs', source_directories=('/foo', '/bar'))
+
+
+def test_get_get_all_datasets_does_not_filter_datasets():
+    flexmock(borgmatic.execute).should_receive('execute_command_and_capture_output').and_return(
+        'dataset\t/dataset\nother\t/other',
+    )
+
+    assert module.get_all_datasets('zfs') == (
+        ('dataset', '/dataset'),
+        ('other', '/other'),
+    )
+
+
+def test_get_all_datasets_with_invalid_list_output_raises():
+    flexmock(borgmatic.execute).should_receive('execute_command_and_capture_output').and_return(
+        'dataset',
+    )
+
+    with pytest.raises(ValueError, match='zfs'):
+        module.get_all_datasets('zfs')
+
+
+def test_dump_data_sources_snapshots_and_mounts_and_updates_source_directories():
+    flexmock(module).should_receive('get_datasets_to_backup').and_return(
+        (('dataset', '/mnt/dataset'),)
+    )
+    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 = ['/mnt/dataset']
+
+    assert (
+        module.dump_data_sources(
+            hook_config={},
+            config={'source_directories': '/mnt/dataset', 'zfs': {}},
+            log_prefix='test',
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=False,
+        )
+        == []
+    )
+
+    assert source_directories == [snapshot_mount_path]
+
+
+def test_dump_data_sources_uses_custom_commands():
+    flexmock(module).should_receive('get_datasets_to_backup').and_return(
+        (('dataset', '/mnt/dataset'),)
+    )
+    flexmock(module.os).should_receive('getpid').and_return(1234)
+    full_snapshot_name = 'dataset@borgmatic-1234'
+    flexmock(module).should_receive('snapshot_dataset').with_args(
+        '/usr/local/bin/zfs',
+        full_snapshot_name,
+    ).once()
+    snapshot_mount_path = '/run/borgmatic/zfs_snapshots/./mnt/dataset'
+    flexmock(module).should_receive('mount_snapshot').with_args(
+        '/usr/local/bin/mount',
+        full_snapshot_name,
+        module.os.path.normpath(snapshot_mount_path),
+    ).once()
+    source_directories = ['/mnt/dataset']
+    hook_config = {
+        'zfs_command': '/usr/local/bin/zfs',
+        'mount_command': '/usr/local/bin/mount',
+    }
+
+    assert (
+        module.dump_data_sources(
+            hook_config=hook_config,
+            config={
+                'source_directories': source_directories,
+                'zfs': hook_config,
+            },
+            log_prefix='test',
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=False,
+        )
+        == []
+    )
+
+    assert source_directories == [snapshot_mount_path]
+
+
+def test_dump_data_sources_with_dry_run_skips_commands_and_does_not_touch_source_directories():
+    flexmock(module).should_receive('get_datasets_to_backup').and_return(
+        (('dataset', '/mnt/dataset'),)
+    )
+    flexmock(module.os).should_receive('getpid').and_return(1234)
+    flexmock(module).should_receive('snapshot_dataset').never()
+    flexmock(module).should_receive('mount_snapshot').never()
+    source_directories = ['/mnt/dataset']
+
+    assert (
+        module.dump_data_sources(
+            hook_config={},
+            config={'source_directories': '/mnt/dataset', 'zfs': {}},
+            log_prefix='test',
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=True,
+        )
+        == []
+    )
+
+    assert source_directories == ['/mnt/dataset']
+
+
+def test_get_all_snapshots_parses_list_output():
+    flexmock(borgmatic.execute).should_receive('execute_command_and_capture_output').and_return(
+        'dataset1@borgmatic-1234\ndataset2@borgmatic-4567',
+    )
+
+    assert module.get_all_snapshots('zfs') == ('dataset1@borgmatic-1234', 'dataset2@borgmatic-4567')
+
+
+def test_remove_data_source_dumps_unmounts_and_destroys_snapshots():
+    flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/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').and_return(True)
+    flexmock(module.shutil).should_receive('rmtree')
+    flexmock(module).should_receive('unmount_snapshot').with_args(
+        'umount', '/run/borgmatic/zfs_snapshots/mnt/dataset'
+    ).once()
+    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_use_custom_commands():
+    flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/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').and_return(True)
+    flexmock(module.shutil).should_receive('rmtree')
+    flexmock(module).should_receive('unmount_snapshot').with_args(
+        '/usr/local/bin/umount', '/run/borgmatic/zfs_snapshots/mnt/dataset'
+    ).once()
+    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(
+        '/usr/local/bin/zfs', 'dataset@borgmatic-1234'
+    ).once()
+    hook_config = {'zfs_command': '/usr/local/bin/zfs', 'umount_command': '/usr/local/bin/umount'}
+
+    module.remove_data_source_dumps(
+        hook_config=hook_config,
+        config={'source_directories': '/mnt/dataset', 'zfs': hook_config},
+        log_prefix='test',
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=False,
+    )
+
+
+def test_remove_data_source_dumps_bails_for_missing_zfs_command():
+    flexmock(module).should_receive('get_all_datasets').and_raise(FileNotFoundError)
+    flexmock(module.borgmatic.config.paths).should_receive(
+        'replace_temporary_subdirectory_with_glob'
+    ).never()
+    hook_config = {'zfs_command': 'wtf'}
+
+    module.remove_data_source_dumps(
+        hook_config=hook_config,
+        config={'source_directories': '/mnt/dataset', 'zfs': hook_config},
+        log_prefix='test',
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=False,
+    )
+
+
+def test_remove_data_source_dumps_bails_for_zfs_command_error():
+    flexmock(module).should_receive('get_all_datasets').and_raise(
+        module.subprocess.CalledProcessError(1, 'wtf')
+    )
+    flexmock(module.borgmatic.config.paths).should_receive(
+        'replace_temporary_subdirectory_with_glob'
+    ).never()
+    hook_config = {'zfs_command': 'wtf'}
+
+    module.remove_data_source_dumps(
+        hook_config=hook_config,
+        config={'source_directories': '/mnt/dataset', 'zfs': hook_config},
+        log_prefix='test',
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=False,
+    )
+
+
+def test_remove_data_source_dumps_skips_unmount_snapshot_directories_that_are_not_actually_directories():
+    flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/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').and_return(False)
+    flexmock(module.shutil).should_receive('rmtree').never()
+    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_skips_unmount_snapshot_mount_paths_that_are_not_actually_directories():
+    flexmock(module).should_receive('get_all_datasets').and_return((('dataset', '/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(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_datasets').and_return((('dataset', '/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').and_return(True)
+    flexmock(module.shutil).should_receive('rmtree').never()
+    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').never()
+
+    module.remove_data_source_dumps(
+        hook_config={},
+        config={'source_directories': '/mnt/dataset', 'zfs': {}},
+        log_prefix='test',
+        borgmatic_runtime_directory='/run/borgmatic',
+        dry_run=True,
+    )