Get all existing tests passing (#962).

borgmatic/actions/create.py

@@ -1,5 +1,3 @@
-import collections
-import enum
 import glob
 import itertools
 import logging
@@ -135,7 +133,7 @@ def expand_patterns(patterns, working_directory=None, skip_paths=None):
                     for expanded_path in expand_directory(pattern.path, working_directory)
                 )
                 if pattern.type == borgmatic.borg.pattern.Pattern_type.ROOT
-                and pattern.path not in skip_paths
+                and pattern.path not in (skip_paths or ())
                 else (pattern,)
             )
             for pattern in patterns
@@ -188,11 +186,11 @@ def deduplicate_patterns(patterns):
     even hangs, e.g. when a database hook is using a named pipe for streaming database dumps to
     Borg.
     '''
-    deduplicated = []
+    deduplicated = {}  # Use just the keys as an ordered set.
 
     for pattern in patterns:
         if pattern.type != borgmatic.borg.pattern.Pattern_type.ROOT:
-            deduplicated.append(pattern)
+            deduplicated[pattern] = None
             continue
 
         parents = pathlib.PurePath(pattern.path).parents
@@ -203,17 +201,17 @@ def deduplicate_patterns(patterns):
             if other_pattern.type != borgmatic.borg.pattern.Pattern_type.ROOT:
                 continue
 
-            for parent in parents:
-                if (
-                    pathlib.PurePath(other_pattern.path) == parent
-                    and pattern.device is not None
-                    and other_pattern.device == pattern.device
-                ):
-                    break
+            if any(
+                pathlib.PurePath(other_pattern.path) == parent
+                and pattern.device is not None
+                and other_pattern.device == pattern.device
+                for parent in parents
+            ):
+                break
         else:
-            deduplicated.append(pattern)
+            deduplicated[pattern] = None
 
-    return tuple(deduplicated)
+    return tuple(deduplicated.keys())
 
 
 def process_patterns(patterns, working_directory, skip_expand_paths=None):

borgmatic/config/schema.yaml

@@ -144,8 +144,8 @@ properties:
             type: string
         description: |
             Read include/exclude patterns from one or more separate named files,
-            one pattern per line. See the output of "borg help patterns" for more
-            details.
+            one pattern per line. See the output of "borg help patterns" for
+            more details.
         example:
             - /etc/borgmatic/patterns
     exclude_patterns:

borgmatic/hooks/data_source/snapshot.py

@@ -1,8 +1,5 @@
 import pathlib
 
-import borgmatic.borg.pattern
-
-
 IS_A_HOOK = False
 
 

tests/unit/actions/test_check.py

@@ -1,8 +1,8 @@
 import pytest
 from flexmock import flexmock
 
-from borgmatic.borg.pattern import Pattern
 from borgmatic.actions import check as module
+from borgmatic.borg.pattern import Pattern
 
 
 def test_parse_checks_returns_them_as_tuple():

tests/unit/actions/test_create.py

@@ -2,6 +2,7 @@ import pytest
 from flexmock import flexmock
 
 from borgmatic.actions import create as module
+from borgmatic.borg.pattern import Pattern, Pattern_type
 
 
 def test_expand_directory_with_basic_path_passes_it_through():
@@ -53,7 +54,7 @@ def test_expand_directory_with_slashdot_hack_globs_working_directory_and_strips_
     assert paths == ['./foo', './food']
 
 
-def test_expand_directories_flattens_expanded_directories():
+def test_expand_patterns_flattens_expanded_directories():
     flexmock(module).should_receive('expand_directory').with_args('~/foo', None).and_return(
         ['/root/foo']
     )
@@ -61,167 +62,205 @@ def test_expand_directories_flattens_expanded_directories():
         ['bar', 'barf']
     )
 
-    paths = module.expand_directories(('~/foo', 'bar*'))
+    paths = module.expand_patterns((Pattern('~/foo'), Pattern('bar*')))
 
-    assert paths == ('/root/foo', 'bar', 'barf')
+    assert paths == (Pattern('/root/foo'), Pattern('bar'), Pattern('barf'))
 
 
-def test_expand_directories_with_working_directory_passes_it_through():
+def test_expand_patterns_with_working_directory_passes_it_through():
     flexmock(module).should_receive('expand_directory').with_args('foo', '/working/dir').and_return(
         ['/working/dir/foo']
     )
 
-    paths = module.expand_directories(('foo',), working_directory='/working/dir')
+    patterns = module.expand_patterns((Pattern('foo'),), working_directory='/working/dir')
 
-    assert paths == ('/working/dir/foo',)
+    assert patterns == (Pattern('/working/dir/foo'),)
 
 
-def test_expand_directories_considers_none_as_no_directories():
-    paths = module.expand_directories(None, None)
+def test_expand_patterns_does_not_expand_skip_paths():
+    flexmock(module).should_receive('expand_directory').with_args('/foo', None).and_return(['/foo'])
+    flexmock(module).should_receive('expand_directory').with_args('/bar*', None).never()
 
-    assert paths == ()
+    patterns = module.expand_patterns((Pattern('/foo'), Pattern('/bar*')), skip_paths=('/bar*',))
 
+    assert patterns == (Pattern('/foo'), Pattern('/bar*'))
 
-def test_map_directories_to_devices_gives_device_id_per_path():
+
+def test_expand_patterns_considers_none_as_no_patterns():
+    assert module.expand_patterns(None) == ()
+
+
+def test_expand_patterns_only_considers_root_patterns():
+    flexmock(module).should_receive('expand_directory').with_args('~/foo', None).and_return(
+        ['/root/foo']
+    )
+    flexmock(module).should_receive('expand_directory').with_args('bar*', None).never()
+
+    paths = module.expand_patterns((Pattern('~/foo'), Pattern('bar*', Pattern_type.INCLUDE)))
+
+    assert paths == (Pattern('/root/foo'), Pattern('bar*', Pattern_type.INCLUDE))
+
+
+def test_device_map_patterns_gives_device_id_per_path():
     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').and_return(flexmock(st_dev=66))
 
-    device_map = module.map_directories_to_devices(('/foo', '/bar'))
+    device_map = module.device_map_patterns((Pattern('/foo'), Pattern('/bar')))
 
-    assert device_map == {
-        '/foo': 55,
-        '/bar': 66,
-    }
+    assert device_map == (
+        Pattern('/foo', device=55),
+        Pattern('/bar', device=66),
+    )
 
 
-def test_map_directories_to_devices_with_missing_path_does_not_error():
+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))
     flexmock(module.os).should_receive('stat').with_args('/bar').never()
 
-    device_map = module.map_directories_to_devices(('/foo', '/bar'))
+    device_map = module.device_map_patterns((Pattern('/foo'), Pattern('/bar')))
 
-    assert device_map == {
-        '/foo': 55,
-        '/bar': None,
-    }
+    assert device_map == (
+        Pattern('/foo', device=55),
+        Pattern('/bar'),
+    )
 
 
-def test_map_directories_to_devices_uses_working_directory_to_construct_path():
+def test_device_map_patterns_uses_working_directory_to_construct_path():
     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('/working/dir/bar').and_return(
         flexmock(st_dev=66)
     )
 
-    device_map = module.map_directories_to_devices(
-        ('/foo', 'bar'), working_directory='/working/dir'
+    device_map = module.device_map_patterns(
+        (Pattern('/foo'), Pattern('bar')), working_directory='/working/dir'
     )
 
-    assert device_map == {
-        '/foo': 55,
-        'bar': 66,
-    }
-
-
-@pytest.mark.parametrize(
-    'directories,additional_directories,expected_directories',
-    (
-        ({'/': 1, '/root': 1}, {}, ['/']),
-        ({'/': 1, '/root/': 1}, {}, ['/']),
-        ({'/': 1, '/root': 2}, {}, ['/', '/root']),
-        ({'/root': 1, '/': 1}, {}, ['/']),
-        ({'/root': 1, '/root/foo': 1}, {}, ['/root']),
-        ({'/root/': 1, '/root/foo': 1}, {}, ['/root/']),
-        ({'/root': 1, '/root/foo/': 1}, {}, ['/root']),
-        ({'/root': 1, '/root/foo': 2}, {}, ['/root', '/root/foo']),
-        ({'/root/foo': 1, '/root': 1}, {}, ['/root']),
-        ({'/root': None, '/root/foo': None}, {}, ['/root', '/root/foo']),
-        ({'/root': 1, '/etc': 1, '/root/foo/bar': 1}, {}, ['/etc', '/root']),
-        ({'/root': 1, '/root/foo': 1, '/root/foo/bar': 1}, {}, ['/root']),
-        ({'/dup': 1, '/dup': 1}, {}, ['/dup']),
-        ({'/foo': 1, '/bar': 1}, {}, ['/bar', '/foo']),
-        ({'/foo': 1, '/bar': 2}, {}, ['/bar', '/foo']),
-        ({'/root/foo': 1}, {'/root': 1}, []),
-        ({'/root/foo': 1}, {'/root': 2}, ['/root/foo']),
-        ({'/root/foo': 1}, {}, ['/root/foo']),
-    ),
-)
-def test_deduplicate_directories_removes_child_paths_on_the_same_filesystem(
-    directories, additional_directories, expected_directories
-):
-    assert (
-        module.deduplicate_directories(directories, additional_directories) == expected_directories
+    assert device_map == (
+        Pattern('/foo', device=55),
+        Pattern('bar', device=66),
     )
 
 
-def test_pattern_root_directories_deals_with_none_patterns():
-    assert module.pattern_root_directories(patterns=None) == []
-
-
-def test_pattern_root_directories_parses_roots_and_ignores_others():
-    assert module.pattern_root_directories(
-        ['R /root', '+ /root/foo', '- /root/foo/bar', 'R /baz']
-    ) == ['/root', '/baz']
+def test_device_map_patterns_with_existing_device_id_does_not_overwrite_it():
+    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').and_return(flexmock(st_dev=100))
 
+    device_map = module.device_map_patterns((Pattern('/foo'), Pattern('/bar', device=66)))
 
-def test_process_source_directories_includes_source_directories():
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(
-        '/working'
+    assert device_map == (
+        Pattern('/foo', device=55),
+        Pattern('/bar', device=66),
     )
-    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']},
-    ) == ('foo', 'bar')
+
+@pytest.mark.parametrize(
+    'patterns,expected_patterns',
+    (
+        ((Pattern('/', device=1), Pattern('/root', device=1)), (Pattern('/', device=1),)),
+        ((Pattern('/', device=1), Pattern('/root/', device=1)), (Pattern('/', device=1),)),
+        (
+            (Pattern('/', device=1), Pattern('/root', device=2)),
+            (Pattern('/', device=1), Pattern('/root', device=2)),
+        ),
+        ((Pattern('/root', device=1), Pattern('/', device=1)), (Pattern('/', device=1),)),
+        (
+            (Pattern('/root', device=1), Pattern('/root/foo', device=1)),
+            (Pattern('/root', device=1),),
+        ),
+        (
+            (Pattern('/root/', device=1), Pattern('/root/foo', device=1)),
+            (Pattern('/root/', device=1),),
+        ),
+        (
+            (Pattern('/root', device=1), Pattern('/root/foo/', device=1)),
+            (Pattern('/root', device=1),),
+        ),
+        (
+            (Pattern('/root', device=1), Pattern('/root/foo', device=2)),
+            (Pattern('/root', device=1), Pattern('/root/foo', device=2)),
+        ),
+        (
+            (Pattern('/root/foo', device=1), Pattern('/root', device=1)),
+            (Pattern('/root', device=1),),
+        ),
+        (
+            (Pattern('/root', device=None), Pattern('/root/foo', device=None)),
+            (Pattern('/root'), Pattern('/root/foo')),
+        ),
+        (
+            (
+                Pattern('/root', device=1),
+                Pattern('/etc', device=1),
+                Pattern('/root/foo/bar', device=1),
+            ),
+            (Pattern('/root', device=1), Pattern('/etc', device=1)),
+        ),
+        (
+            (
+                Pattern('/root', device=1),
+                Pattern('/root/foo', device=1),
+                Pattern('/root/foo/bar', device=1),
+            ),
+            (Pattern('/root', device=1),),
+        ),
+        ((Pattern('/dup', device=1), Pattern('/dup', device=1)), (Pattern('/dup', device=1),)),
+        (
+            (Pattern('/foo', device=1), Pattern('/bar', device=1)),
+            (Pattern('/foo', device=1), Pattern('/bar', device=1)),
+        ),
+        (
+            (Pattern('/foo', device=1), Pattern('/bar', device=2)),
+            (Pattern('/foo', device=1), Pattern('/bar', device=2)),
+        ),
+        ((Pattern('/root/foo', device=1),), (Pattern('/root/foo', device=1),)),
+        (
+            (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)),
+            (Pattern('/', device=1), Pattern('/root', Pattern_type.INCLUDE, device=1)),
+        ),
+    ),
+)
+def test_deduplicate_patterns_omits_child_paths_on_the_same_filesystem(patterns, expected_patterns):
+    assert module.deduplicate_patterns(patterns) == expected_patterns
 
 
-def test_process_source_directories_prefers_source_directory_argument_to_config():
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(
-        '/working'
+def test_process_patterns_includes_patterns():
+    flexmock(module).should_receive('deduplicate_patterns').and_return(
+        (Pattern('foo'), Pattern('bar'))
     )
-    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'
+    flexmock(module).should_receive('device_map_patterns').and_return({})
+    flexmock(module).should_receive('expand_patterns').with_args(
+        (Pattern('foo'), Pattern('bar')),
+        working_directory='/working',
+        skip_paths=set(),
     ).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': ['nope']},
-        source_directories=['foo', 'bar'],
-    ) == ('foo', 'bar')
+    assert module.process_patterns(
+        (Pattern('foo'), Pattern('bar')),
+        working_directory='/working',
+    ) == [Pattern('foo'), Pattern('bar')]
 
 
-def test_process_source_directories_skips_expand_for_requested_paths():
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(
-        '/working'
+def test_process_patterns_skips_expand_for_requested_paths():
+    skip_paths = {flexmock()}
+    flexmock(module).should_receive('deduplicate_patterns').and_return(
+        (Pattern('foo'), Pattern('bar'))
     )
-    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(
-        ('bar',), working_directory='/working'
+    flexmock(module).should_receive('device_map_patterns').and_return({})
+    flexmock(module).should_receive('expand_patterns').with_args(
+        (Pattern('foo'), Pattern('bar')),
+        working_directory='/working',
+        skip_paths=skip_paths,
     ).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']}, skip_expand_paths=('foo',)
-    ) == ('foo', 'bar')
+    assert module.process_patterns(
+        (Pattern('foo'), Pattern('bar')),
+        working_directory='/working',
+        skip_expand_paths=skip_paths,
+    ) == [Pattern('foo'), Pattern('bar')]
 
 
 def test_run_create_executes_and_calls_hooks_for_configured_repository():
@@ -236,7 +275,7 @@ def test_run_create_executes_and_calls_hooks_for_configured_repository():
     flexmock(module.borgmatic.hooks.dispatch).should_receive(
         'call_hooks_even_if_unconfigured'
     ).and_return({})
-    flexmock(module).should_receive('process_source_directories').and_return([])
+    flexmock(module).should_receive('process_patterns').and_return([])
     flexmock(module.os.path).should_receive('join').and_return('/run/borgmatic/bootstrap')
     create_arguments = flexmock(
         repository=None,
@@ -278,7 +317,7 @@ def test_run_create_runs_with_selected_repository():
     flexmock(module.borgmatic.hooks.dispatch).should_receive(
         'call_hooks_even_if_unconfigured'
     ).and_return({})
-    flexmock(module).should_receive('process_source_directories').and_return([])
+    flexmock(module).should_receive('process_patterns').and_return([])
     flexmock(module.os.path).should_receive('join').and_return('/run/borgmatic/bootstrap')
     create_arguments = flexmock(
         repository=flexmock(),
@@ -357,7 +396,7 @@ def test_run_create_produces_json():
     flexmock(module.borgmatic.hooks.dispatch).should_receive(
         'call_hooks_even_if_unconfigured'
     ).and_return({})
-    flexmock(module).should_receive('process_source_directories').and_return([])
+    flexmock(module).should_receive('process_patterns').and_return([])
     flexmock(module.os.path).should_receive('join').and_return('/run/borgmatic/bootstrap')
     create_arguments = flexmock(
         repository=flexmock(),

tests/unit/borg/test_create.py

@@ -1,5 +1,4 @@
 import logging
-import sys
 
 import pytest
 from flexmock import flexmock

tests/unit/hooks/data_source/test_btrfs.py

@@ -1,7 +1,7 @@
 import pytest
 from flexmock import flexmock
 
-from borgmatic.borg.pattern import Pattern, Pattern_type, Pattern_style
+from borgmatic.borg.pattern import Pattern, Pattern_style, Pattern_type
 from borgmatic.hooks.data_source import btrfs as module
 
 

tests/unit/hooks/data_source/test_sqlite.py

@@ -2,7 +2,6 @@ import logging
 
 from flexmock import flexmock
 
-from borgmatic.borg.pattern import Pattern
 from borgmatic.hooks.data_source import sqlite as module