From 2467518d4e56e81846c4ea9c3ccb7d8a3cd51e4e Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 16 Jan 2025 15:11:59 -0800 Subject: [PATCH] Add even more missing test coverage (#962). --- borgmatic/actions/check.py | 2 +- borgmatic/actions/create.py | 4 +- borgmatic/borg/create.py | 4 +- borgmatic/borg/pattern.py | 2 +- tests/unit/actions/test_create.py | 19 ++++++ tests/unit/borg/test_create.py | 10 +++- tests/unit/hooks/data_source/test_btrfs.py | 38 +++++++++--- tests/unit/hooks/data_source/test_lvm.py | 60 ++++++++++++++++++- tests/unit/hooks/data_source/test_snapshot.py | 7 +++ tests/unit/hooks/data_source/test_zfs.py | 49 +++++++++++++-- 10 files changed, 173 insertions(+), 22 deletions(-) diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index 472dabe6..5e7b401d 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -634,7 +634,7 @@ def spot_check( f'{log_prefix}: Paths in latest archive but not source paths: {", ".join(set(archive_paths)) or "none"}' ) raise ValueError( - f'Spot check failed: There are no source paths to compare against the archive' + 'Spot check failed: There are no source paths to compare against the archive' ) # Calculate the percentage delta between the source paths count and the archive paths count, and diff --git a/borgmatic/actions/create.py b/borgmatic/actions/create.py index 43772d28..e26bca56 100644 --- a/borgmatic/actions/create.py +++ b/borgmatic/actions/create.py @@ -203,7 +203,7 @@ def deduplicate_patterns(patterns): for pattern in patterns: if pattern.type != borgmatic.borg.pattern.Pattern_type.ROOT: - deduplicated[pattern] = None + deduplicated[pattern] = True continue parents = pathlib.PurePath(pattern.path).parents @@ -222,7 +222,7 @@ def deduplicate_patterns(patterns): ): break else: - deduplicated[pattern] = None + deduplicated[pattern] = True return tuple(deduplicated.keys()) diff --git a/borgmatic/borg/create.py b/borgmatic/borg/create.py index 02b3c5a5..c0f4399c 100644 --- a/borgmatic/borg/create.py +++ b/borgmatic/borg/create.py @@ -208,7 +208,7 @@ def make_base_create_command( stream_processes=None, ): ''' - Given vebosity/dry-run flags, a local or remote repository path, a configuration dict, a + Given verbosity/dry-run flags, a local or remote repository path, a configuration dict, a sequence of patterns as borgmatic.borg.pattern.Pattern instances, the local Borg version, global arguments as an argparse.Namespace instance, and a sequence of borgmatic source directories, return a tuple of (base Borg create command flags, Borg create command positional @@ -361,7 +361,7 @@ def create_archive( stream_processes=None, ): ''' - Given vebosity/dry-run flags, a local or remote repository path, a configuration dict, a + Given verbosity/dry-run flags, a local or remote repository path, a configuration dict, a sequence of loaded configuration paths, the local Borg version, and global arguments as an argparse.Namespace instance, create a Borg archive and return Borg's JSON output (if any). diff --git a/borgmatic/borg/pattern.py b/borgmatic/borg/pattern.py index 1a3f11d8..be57bc0e 100644 --- a/borgmatic/borg/pattern.py +++ b/borgmatic/borg/pattern.py @@ -4,7 +4,7 @@ import enum # See https://borgbackup.readthedocs.io/en/stable/usage/help.html#borg-help-patterns class Pattern_type(enum.Enum): - ROOT = 'R' # A ROOT pattern type always has a NONE pattern style. + ROOT = 'R' # A ROOT pattern always has a NONE pattern style. PATTERN_STYLE = 'P' EXCLUDE = '-' NO_RECURSE = '!' diff --git a/tests/unit/actions/test_create.py b/tests/unit/actions/test_create.py index 42b68f51..ae197c05 100644 --- a/tests/unit/actions/test_create.py +++ b/tests/unit/actions/test_create.py @@ -221,6 +221,21 @@ def test_device_map_patterns_gives_device_id_per_path(): ) +def test_device_map_patterns_only_considers_root_patterns(): + flexmock(module.os.path).should_receive('exists').and_return(True) + flexmock(module.os).should_receive('stat').with_args('/foo').and_return(flexmock(st_dev=55)) + flexmock(module.os).should_receive('stat').with_args('/bar*').never() + + device_map = module.device_map_patterns( + (Pattern('/foo'), Pattern('/bar*', Pattern_type.INCLUDE)) + ) + + assert device_map == ( + Pattern('/foo', device=55), + Pattern('/bar*', Pattern_type.INCLUDE), + ) + + def test_device_map_patterns_with_missing_path_does_not_error(): flexmock(module.os.path).should_receive('exists').and_return(True).and_return(False) flexmock(module.os).should_receive('stat').with_args('/foo').and_return(flexmock(st_dev=55)) @@ -328,6 +343,10 @@ def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it(): (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)), (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)), ), + ( + (Pattern('/root', Pattern_type.INCLUDE, device=1), Pattern('/', device=1)), + (Pattern('/root', Pattern_type.INCLUDE, device=1), Pattern('/', device=1)), + ), ), ) def test_deduplicate_patterns_omits_child_paths_on_the_same_filesystem(patterns, expected_patterns): diff --git a/tests/unit/borg/test_create.py b/tests/unit/borg/test_create.py index 2df4ebed..5e3368e9 100644 --- a/tests/unit/borg/test_create.py +++ b/tests/unit/borg/test_create.py @@ -11,11 +11,11 @@ from ..test_verbosity import insert_logging_mock def test_write_patterns_file_writes_pattern_lines(): temporary_file = flexmock(name='filename', flush=lambda: None) - temporary_file.should_receive('write').with_args('R /foo\n+ /foo/bar') + temporary_file.should_receive('write').with_args('R /foo\n+ sh:/foo/bar') flexmock(module.tempfile).should_receive('NamedTemporaryFile').and_return(temporary_file) module.write_patterns_file( - [Pattern('/foo'), Pattern('/foo/bar', Pattern_type.INCLUDE)], + [Pattern('/foo'), Pattern('/foo/bar', Pattern_type.INCLUDE, Pattern_style.SHELL)], borgmatic_runtime_directory='/run/user/0', log_prefix='test.yaml', ) @@ -1578,6 +1578,12 @@ def test_check_all_root_patterns_exist_with_existent_pattern_path_does_not_raise module.check_all_root_patterns_exist([Pattern('foo')]) +def test_check_all_root_patterns_exist_with_non_root_pattern_skips_existence_check(): + flexmock(module.os.path).should_receive('exists').never() + + module.check_all_root_patterns_exist([Pattern('foo', Pattern_type.INCLUDE)]) + + def test_check_all_root_patterns_exist_with_non_existent_pattern_path_raises(): flexmock(module.os.path).should_receive('exists').and_return(False) diff --git a/tests/unit/hooks/data_source/test_btrfs.py b/tests/unit/hooks/data_source/test_btrfs.py index 45d3d1ad..72f77f82 100644 --- a/tests/unit/hooks/data_source/test_btrfs.py +++ b/tests/unit/hooks/data_source/test_btrfs.py @@ -154,22 +154,42 @@ def test_make_snapshot_path_includes_stripped_subvolume_path( @pytest.mark.parametrize( - 'subvolume_path,pattern_path,expected_path', + 'subvolume_path,pattern,expected_pattern', ( - ('/foo/bar', '/foo/bar/baz', '/foo/bar/.borgmatic-snapshot-1234/./foo/bar/baz'), - ('/foo/bar', '/foo/bar', '/foo/bar/.borgmatic-snapshot-1234/./foo/bar'), - ('/', '/foo', '/.borgmatic-snapshot-1234/./foo'), - ('/', '/', '/.borgmatic-snapshot-1234/./'), + ( + '/foo/bar', + Pattern('/foo/bar/baz'), + Pattern('/foo/bar/.borgmatic-snapshot-1234/./foo/bar/baz'), + ), + ('/foo/bar', Pattern('/foo/bar'), Pattern('/foo/bar/.borgmatic-snapshot-1234/./foo/bar')), + ( + '/foo/bar', + Pattern('^/foo/bar', Pattern_type.INCLUDE, Pattern_style.REGULAR_EXPRESSION), + Pattern( + '^/foo/bar/.borgmatic-snapshot-1234/./foo/bar', + Pattern_type.INCLUDE, + Pattern_style.REGULAR_EXPRESSION, + ), + ), + ( + '/foo/bar', + Pattern('/foo/bar', Pattern_type.INCLUDE, Pattern_style.REGULAR_EXPRESSION), + Pattern( + '/foo/bar/.borgmatic-snapshot-1234/./foo/bar', + Pattern_type.INCLUDE, + Pattern_style.REGULAR_EXPRESSION, + ), + ), + ('/', Pattern('/foo'), Pattern('/.borgmatic-snapshot-1234/./foo')), + ('/', Pattern('/'), Pattern('/.borgmatic-snapshot-1234/./')), ), ) def test_make_borg_snapshot_pattern_includes_slashdot_hack_and_stripped_pattern_path( - subvolume_path, pattern_path, expected_path + subvolume_path, pattern, expected_pattern ): flexmock(module.os).should_receive('getpid').and_return(1234) - assert module.make_borg_snapshot_pattern(subvolume_path, Pattern(pattern_path)) == Pattern( - expected_path - ) + assert module.make_borg_snapshot_pattern(subvolume_path, pattern) == expected_pattern def test_dump_data_sources_snapshots_each_subvolume_and_updates_patterns(): diff --git a/tests/unit/hooks/data_source/test_lvm.py b/tests/unit/hooks/data_source/test_lvm.py index 710c7205..6b06cefc 100644 --- a/tests/unit/hooks/data_source/test_lvm.py +++ b/tests/unit/hooks/data_source/test_lvm.py @@ -1,7 +1,7 @@ import pytest from flexmock import flexmock -from borgmatic.borg.pattern import Pattern +from borgmatic.borg.pattern import Pattern, Pattern_style, Pattern_type from borgmatic.hooks.data_source import lvm as module @@ -133,6 +133,40 @@ def test_snapshot_logical_volume_with_non_percentage_snapshot_name_uses_lvcreate module.snapshot_logical_volume('lvcreate', 'snap', '/dev/snap', '10TB') +@pytest.mark.parametrize( + 'pattern,expected_pattern', + ( + ( + Pattern('/foo/bar/baz'), + Pattern('/run/borgmatic/lvm_snapshots/./foo/bar/baz'), + ), + (Pattern('/foo/bar'), Pattern('/run/borgmatic/lvm_snapshots/./foo/bar')), + ( + Pattern('^/foo/bar', Pattern_type.INCLUDE, Pattern_style.REGULAR_EXPRESSION), + Pattern( + '^/run/borgmatic/lvm_snapshots/./foo/bar', + Pattern_type.INCLUDE, + Pattern_style.REGULAR_EXPRESSION, + ), + ), + ( + Pattern('/foo/bar', Pattern_type.INCLUDE, Pattern_style.REGULAR_EXPRESSION), + Pattern( + '/run/borgmatic/lvm_snapshots/./foo/bar', + Pattern_type.INCLUDE, + Pattern_style.REGULAR_EXPRESSION, + ), + ), + (Pattern('/foo'), Pattern('/run/borgmatic/lvm_snapshots/./foo')), + (Pattern('/'), Pattern('/run/borgmatic/lvm_snapshots/./')), + ), +) +def test_make_borg_snapshot_pattern_includes_slashdot_hack_and_stripped_pattern_path( + pattern, expected_pattern +): + assert module.make_borg_snapshot_pattern(pattern, '/run/borgmatic') == expected_pattern + + def test_dump_data_sources_snapshots_and_mounts_and_updates_patterns(): config = {'lvm': {}} patterns = [Pattern('/mnt/lvolume1/subdir'), Pattern('/mnt/lvolume2')] @@ -175,6 +209,12 @@ def test_dump_data_sources_snapshots_and_mounts_and_updates_patterns(): flexmock(module).should_receive('mount_snapshot').with_args( 'mount', '/dev/lvolume2_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume2' ).once() + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/lvolume1/subdir'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/lvm_snapshots/./mnt/lvolume1/subdir')) + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/lvolume2'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/lvm_snapshots/./mnt/lvolume2')) assert ( module.dump_data_sources( @@ -266,6 +306,12 @@ def test_dump_data_sources_uses_snapshot_size_for_snapshot(): flexmock(module).should_receive('mount_snapshot').with_args( 'mount', '/dev/lvolume2_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume2' ).once() + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/lvolume1/subdir'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/lvm_snapshots/./mnt/lvolume1/subdir')) + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/lvolume2'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/lvm_snapshots/./mnt/lvolume2')) assert ( module.dump_data_sources( @@ -341,6 +387,12 @@ def test_dump_data_sources_uses_custom_commands(): flexmock(module).should_receive('mount_snapshot').with_args( '/usr/local/bin/mount', '/dev/lvolume2_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume2' ).once() + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/lvolume1/subdir'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/lvm_snapshots/./mnt/lvolume1/subdir')) + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/lvolume2'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/lvm_snapshots/./mnt/lvolume2')) assert ( module.dump_data_sources( @@ -455,6 +507,12 @@ def test_dump_data_sources_ignores_mismatch_between_given_patterns_and_contained flexmock(module).should_receive('mount_snapshot').with_args( 'mount', '/dev/lvolume2_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume2' ).once() + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/lvolume1/subdir'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/lvm_snapshots/./mnt/lvolume1/subdir')) + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/lvolume2'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/lvm_snapshots/./mnt/lvolume2')) assert ( module.dump_data_sources( diff --git a/tests/unit/hooks/data_source/test_snapshot.py b/tests/unit/hooks/data_source/test_snapshot.py index 1b73e4a1..2c23a1e4 100644 --- a/tests/unit/hooks/data_source/test_snapshot.py +++ b/tests/unit/hooks/data_source/test_snapshot.py @@ -13,6 +13,13 @@ def test_get_contained_patterns_with_self_candidate_returns_self(): assert candidates == {Pattern('/foo'), Pattern('/bar')} +def test_get_contained_patterns_with_self_candidate_and_caret_prefix_returns_self(): + candidates = {Pattern('^/foo'), Pattern('^/mnt'), Pattern('^/bar')} + + assert module.get_contained_patterns('/mnt', candidates) == (Pattern('^/mnt'),) + assert candidates == {Pattern('^/foo'), Pattern('^/bar')} + + def test_get_contained_patterns_with_child_candidate_returns_child(): candidates = {Pattern('/foo'), Pattern('/mnt/subdir'), Pattern('/bar')} diff --git a/tests/unit/hooks/data_source/test_zfs.py b/tests/unit/hooks/data_source/test_zfs.py index 75b9fc76..7700b736 100644 --- a/tests/unit/hooks/data_source/test_zfs.py +++ b/tests/unit/hooks/data_source/test_zfs.py @@ -3,7 +3,7 @@ import os import pytest from flexmock import flexmock -from borgmatic.borg.pattern import Pattern +from borgmatic.borg.pattern import Pattern, Pattern_style, Pattern_type from borgmatic.hooks.data_source import zfs as module @@ -89,6 +89,40 @@ def test_get_all_dataset_mount_points_does_not_filter_datasets(): ) +@pytest.mark.parametrize( + 'pattern,expected_pattern', + ( + ( + Pattern('/foo/bar/baz'), + Pattern('/run/borgmatic/zfs_snapshots/./foo/bar/baz'), + ), + (Pattern('/foo/bar'), Pattern('/run/borgmatic/zfs_snapshots/./foo/bar')), + ( + Pattern('^/foo/bar', Pattern_type.INCLUDE, Pattern_style.REGULAR_EXPRESSION), + Pattern( + '^/run/borgmatic/zfs_snapshots/./foo/bar', + Pattern_type.INCLUDE, + Pattern_style.REGULAR_EXPRESSION, + ), + ), + ( + Pattern('/foo/bar', Pattern_type.INCLUDE, Pattern_style.REGULAR_EXPRESSION), + Pattern( + '/run/borgmatic/zfs_snapshots/./foo/bar', + Pattern_type.INCLUDE, + Pattern_style.REGULAR_EXPRESSION, + ), + ), + (Pattern('/foo'), Pattern('/run/borgmatic/zfs_snapshots/./foo')), + (Pattern('/'), Pattern('/run/borgmatic/zfs_snapshots/./')), + ), +) +def test_make_borg_snapshot_pattern_includes_slashdot_hack_and_stripped_pattern_path( + pattern, expected_pattern +): + assert module.make_borg_snapshot_pattern(pattern, '/run/borgmatic') == expected_pattern + + def test_dump_data_sources_snapshots_and_mounts_and_updates_patterns(): flexmock(module).should_receive('get_datasets_to_backup').and_return( ( @@ -111,6 +145,9 @@ def test_dump_data_sources_snapshots_and_mounts_and_updates_patterns(): full_snapshot_name, module.os.path.normpath(snapshot_mount_path), ).once() + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/dataset/subdir'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/zfs_snapshots/./mnt/dataset/subdir')) patterns = [Pattern('/mnt/dataset/subdir')] assert ( @@ -174,6 +211,9 @@ def test_dump_data_sources_uses_custom_commands(): full_snapshot_name, module.os.path.normpath(snapshot_mount_path), ).once() + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/dataset/subdir'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/zfs_snapshots/./mnt/dataset/subdir')) patterns = [Pattern('/mnt/dataset/subdir')] hook_config = { 'zfs_command': '/usr/local/bin/zfs', @@ -246,9 +286,10 @@ def test_dump_data_sources_ignores_mismatch_between_given_patterns_and_contained full_snapshot_name, module.os.path.normpath(snapshot_mount_path), ).once() - patterns = [ - Pattern('/hmm'), - ] + flexmock(module).should_receive('make_borg_snapshot_pattern').with_args( + Pattern('/mnt/dataset/subdir'), '/run/borgmatic' + ).and_return(Pattern('/run/borgmatic/zfs_snapshots/./mnt/dataset/subdir')) + patterns = [Pattern('/hmm')] assert ( module.dump_data_sources(