Add test coverage for new code.

borgmatic/actions/create.py

@@ -43,44 +43,46 @@ def collect_patterns(config):
     directories, etc., but we'd like to collapse them all down to one common format (patterns) for
     ease of manipulation within borgmatic.
     '''
-    return (
-        tuple(
-            borgmatic.borg.pattern.Pattern(source_directory)
-            for source_directory in config.get('source_directories', ())
-        )
-        + tuple(
-            parse_pattern(pattern_line)
-            for pattern_line in config.get('patterns', ())
-            if not pattern_line.lstrip().startswith('#')
-        )
-        + tuple(
-            borgmatic.borg.pattern.Pattern(
-                exclude_line,
-                borgmatic.borg.pattern.Pattern_type.EXCLUDE,
-                borgmatic.borg.pattern.Pattern_style.FNMATCH,
+    try:
+        return (
+            tuple(
+                borgmatic.borg.pattern.Pattern(source_directory)
+                for source_directory in config.get('source_directories', ())
             )
-            for exclude_line in config.get('exclude_patterns', ())
-        )
-        + tuple(
-            itertools.chain.from_iterable(
-                parse_pattern(pattern_line)
+            + tuple(
+                parse_pattern(pattern_line.strip())
+                for pattern_line in config.get('patterns', ())
+                if not pattern_line.lstrip().startswith('#')
+            )
+            + tuple(
+                borgmatic.borg.pattern.Pattern(
+                    exclude_line.strip(),
+                    borgmatic.borg.pattern.Pattern_type.EXCLUDE,
+                    borgmatic.borg.pattern.Pattern_style.FNMATCH,
+                )
+                for exclude_line in config.get('exclude_patterns', ())
+            )
+            + tuple(
+                parse_pattern(pattern_line.strip())
                 for filename in config.get('patterns_from', ())
                 for pattern_line in open(filename).readlines()
                 if not pattern_line.lstrip().startswith('#')
             )
-        )
-        + tuple(
-            itertools.chain.from_iterable(
+            + tuple(
                 borgmatic.borg.pattern.Pattern(
-                    exclude_line,
+                    exclude_line.strip(),
                     borgmatic.borg.pattern.Pattern_type.EXCLUDE,
-                    borgmatic.borg.pattern.Pattern_type.FNMATCH,
+                    borgmatic.borg.pattern.Pattern_style.FNMATCH,
                 )
                 for filename in config.get('excludes_from', ())
                 for exclude_line in open(filename).readlines()
+                if not exclude_line.lstrip().startswith('#')
             )
         )
-    )
+    except (FileNotFoundError, OSError) as error:
+        logger.debug(error)
+
+        raise ValueError(f'Cannot read patterns_from/excludes_from file: {error.filename}')
 
 
 def expand_directory(directory, working_directory):

borgmatic/borg/create.py

@@ -337,8 +337,9 @@ def make_base_create_command(
                 log_prefix=repository_path,
                 patterns_file=patterns_file,
             )
+
             if '--patterns-from' not in create_flags:
-                create_flags.append(('--patterns-from', patterns_file.name))
+                create_flags += ('--patterns-from', patterns_file.name)
 
     return (create_flags, create_positional_arguments, patterns_file)
 

tests/unit/actions/test_create.py

@@ -1,8 +1,104 @@
+import io
+import sys
+
 import pytest
 from flexmock import flexmock
 
 from borgmatic.actions import create as module
-from borgmatic.borg.pattern import Pattern, Pattern_type
+from borgmatic.borg.pattern import Pattern, Pattern_style, Pattern_type
+
+
+@pytest.mark.parametrize(
+    'pattern_line,expected_pattern',
+    (
+        ('R /foo', Pattern('/foo')),
+        ('P sh', Pattern('sh', Pattern_type.PATTERN_STYLE)),
+        ('+ /foo*', Pattern('/foo*', Pattern_type.INCLUDE)),
+        ('+ sh:/foo*', Pattern('/foo*', Pattern_type.INCLUDE, Pattern_style.SHELL)),
+    ),
+)
+def test_parse_pattern_transforms_pattern_line_to_instance(pattern_line, expected_pattern):
+    module.parse_pattern(pattern_line) == expected_pattern
+
+
+def test_parse_pattern_with_invalid_pattern_line_errors():
+    with pytest.raises(ValueError):
+        module.parse_pattern('/foo')
+
+
+def test_collect_patterns_converts_source_directories():
+    assert module.collect_patterns({'source_directories': ['/foo', '/bar']}) == (
+        Pattern('/foo'),
+        Pattern('/bar'),
+    )
+
+
+def test_collect_patterns_parses_config_patterns():
+    flexmock(module).should_receive('parse_pattern').with_args('R /foo').and_return(Pattern('/foo'))
+    flexmock(module).should_receive('parse_pattern').with_args('# comment').never()
+    flexmock(module).should_receive('parse_pattern').with_args('R /bar').and_return(Pattern('/bar'))
+
+    assert module.collect_patterns({'patterns': ['R /foo', '# comment', 'R /bar']}) == (
+        Pattern('/foo'),
+        Pattern('/bar'),
+    )
+
+
+def test_collect_patterns_converts_exclude_patterns():
+    assert module.collect_patterns({'exclude_patterns': ['/foo', '/bar']}) == (
+        Pattern('/foo', Pattern_type.EXCLUDE, Pattern_style.FNMATCH),
+        Pattern('/bar', Pattern_type.EXCLUDE, Pattern_style.FNMATCH),
+    )
+
+
+def test_collect_patterns_reads_config_patterns_from_file():
+    builtins = flexmock(sys.modules['builtins'])
+    builtins.should_receive('open').with_args('file1.txt').and_return(io.StringIO('R /foo'))
+    builtins.should_receive('open').with_args('file2.txt').and_return(
+        io.StringIO('R /bar\n# comment\nR /baz')
+    )
+    flexmock(module).should_receive('parse_pattern').with_args('R /foo').and_return(Pattern('/foo'))
+    flexmock(module).should_receive('parse_pattern').with_args('# comment').never()
+    flexmock(module).should_receive('parse_pattern').with_args('R /bar').and_return(Pattern('/bar'))
+    flexmock(module).should_receive('parse_pattern').with_args('R /baz').and_return(Pattern('/baz'))
+
+    assert module.collect_patterns({'patterns_from': ['file1.txt', 'file2.txt']}) == (
+        Pattern('/foo'),
+        Pattern('/bar'),
+        Pattern('/baz'),
+    )
+
+
+def test_collect_patterns_errors_on_missing_config_patterns_from_file():
+    builtins = flexmock(sys.modules['builtins'])
+    builtins.should_receive('open').with_args('file1.txt').and_raise(FileNotFoundError)
+    flexmock(module).should_receive('parse_pattern').never()
+
+    with pytest.raises(ValueError):
+        module.collect_patterns({'patterns_from': ['file1.txt', 'file2.txt']})
+
+
+def test_collect_patterns_reads_config_excludes_from_file():
+    builtins = flexmock(sys.modules['builtins'])
+    builtins.should_receive('open').with_args('file1.txt').and_return(io.StringIO('/foo'))
+    builtins.should_receive('open').with_args('file2.txt').and_return(
+        io.StringIO('/bar\n# comment\n/baz')
+    )
+
+    assert module.collect_patterns({'excludes_from': ['file1.txt', 'file2.txt']}) == (
+        Pattern('/foo', Pattern_type.EXCLUDE, Pattern_style.FNMATCH),
+        Pattern('/bar', Pattern_type.EXCLUDE, Pattern_style.FNMATCH),
+        Pattern('/baz', Pattern_type.EXCLUDE, Pattern_style.FNMATCH),
+    )
+
+
+def test_collect_patterns_errors_on_missing_config_excludes_from_file():
+    builtins = flexmock(sys.modules['builtins'])
+    builtins.should_receive('open').with_args('file1.txt').and_raise(OSError)
+    flexmock(module).should_receive('parse_pattern').never()
+
+    with pytest.raises(ValueError):
+        module.collect_patterns({'excludes_from': ['file1.txt', 'file2.txt']})
 
 
 def test_expand_directory_with_basic_path_passes_it_through():
@@ -275,6 +371,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('collect_patterns').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(
@@ -317,6 +414,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('collect_patterns').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(
@@ -396,6 +494,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('collect_patterns').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(

tests/unit/borg/test_create.py

@@ -4,7 +4,7 @@ import pytest
 from flexmock import flexmock
 
 from borgmatic.borg import create as module
-from borgmatic.borg.pattern import Pattern, Pattern_type
+from borgmatic.borg.pattern import Pattern, Pattern_style, Pattern_type
 
 from ..test_verbosity import insert_logging_mock
 
@@ -418,38 +418,6 @@ def test_make_base_create_command_with_store_config_false_omits_config_files():
     assert not pattern_file
 
 
-def test_make_base_create_command_includes_exclude_patterns_in_borg_command():
-    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
-    flexmock(module).should_receive('write_patterns_file').and_return(flexmock(name='patterns'))
-    flexmock(module).should_receive('make_list_filter_flags').and_return('FOO')
-    flexmock(module.flags).should_receive('get_default_archive_name_format').and_return(
-        '{hostname}'
-    )
-    flexmock(module.feature).should_receive('available').and_return(True)
-    flexmock(module).should_receive('make_exclude_flags').and_return(())
-    flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
-        (f'repo::{DEFAULT_ARCHIVE_NAME}',)
-    )
-
-    (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
-        dry_run=False,
-        repository_path='repo',
-        config={
-            'source_directories': ['foo', 'bar'],
-            'repositories': ['repo'],
-            'exclude_patterns': ['exclude'],
-        },
-        patterns=[Pattern('foo'), Pattern('bar')],
-        local_borg_version='1.2.3',
-        global_arguments=flexmock(log_json=False),
-        borgmatic_runtime_directory='/run/borgmatic',
-    )
-
-    assert create_flags == ('borg', 'create', '--patterns-from', 'patterns')
-    assert create_positional_arguments == REPO_ARCHIVE
-    assert pattern_file
-
-
 @pytest.mark.parametrize(
     'option_name,option_value,feature_available,option_flags',
     (
@@ -675,8 +643,12 @@ def test_make_base_create_command_includes_list_flags_in_borg_command():
 
 
 def test_make_base_create_command_with_stream_processes_ignores_read_special_false_and_excludes_special_files():
+    patterns = [Pattern('foo'), Pattern('bar')]
+    patterns_file = flexmock(name='patterns')
     flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
-    flexmock(module).should_receive('write_patterns_file').and_return(None)
+    flexmock(module).should_receive('write_patterns_file').with_args(
+        patterns, '/run/borgmatic', object
+    ).and_return(patterns_file)
     flexmock(module).should_receive('make_list_filter_flags').and_return('FOO')
     flexmock(module.flags).should_receive('get_default_archive_name_format').and_return(
         '{hostname}'
@@ -689,7 +661,18 @@ def test_make_base_create_command_with_stream_processes_ignores_read_special_fal
     flexmock(module.logger).should_receive('warning').twice()
     flexmock(module.environment).should_receive('make_environment')
     flexmock(module).should_receive('collect_special_file_paths').and_return(('/dev/null',)).once()
-    flexmock(module).should_receive('write_patterns_file').and_return(flexmock(name='patterns'))
+    flexmock(module).should_receive('write_patterns_file').with_args(
+        (
+            Pattern(
+                '/dev/null',
+                Pattern_type.EXCLUDE,
+                Pattern_style.FNMATCH,
+            ),
+        ),
+        '/run/borgmatic',
+        'repo',
+        patterns_file=patterns_file,
+    ).and_return(patterns_file).once()
     flexmock(module).should_receive('make_exclude_flags').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
@@ -700,7 +683,7 @@ def test_make_base_create_command_with_stream_processes_ignores_read_special_fal
             'repositories': ['repo'],
             'read_special': False,
         },
-        patterns=[Pattern('foo'), Pattern('bar')],
+        patterns=patterns,
         local_borg_version='1.2.3',
         global_arguments=flexmock(log_json=False),
         borgmatic_runtime_directory='/run/borgmatic',
@@ -712,7 +695,58 @@ def test_make_base_create_command_with_stream_processes_ignores_read_special_fal
     assert pattern_file
 
 
-def test_make_base_create_command_with_stream_processes_and_read_special_true_skip_special_files_excludes():
+def test_make_base_create_command_without_patterns_and_with_stream_processes_ignores_read_special_false_and_excludes_special_files():
+    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
+    flexmock(module).should_receive('write_patterns_file').with_args(
+        [], '/run/borgmatic', object
+    ).and_return(None)
+    flexmock(module).should_receive('make_list_filter_flags').and_return('FOO')
+    flexmock(module.flags).should_receive('get_default_archive_name_format').and_return(
+        '{hostname}'
+    )
+    flexmock(module.feature).should_receive('available').and_return(True)
+    flexmock(module).should_receive('make_exclude_flags').and_return(())
+    flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
+        (f'repo::{DEFAULT_ARCHIVE_NAME}',)
+    )
+    flexmock(module.logger).should_receive('warning').twice()
+    flexmock(module.environment).should_receive('make_environment')
+    flexmock(module).should_receive('collect_special_file_paths').and_return(('/dev/null',)).once()
+    flexmock(module).should_receive('write_patterns_file').with_args(
+        (
+            Pattern(
+                '/dev/null',
+                Pattern_type.EXCLUDE,
+                Pattern_style.FNMATCH,
+            ),
+        ),
+        '/run/borgmatic',
+        'repo',
+        patterns_file=None,
+    ).and_return(flexmock(name='patterns')).once()
+    flexmock(module).should_receive('make_exclude_flags').and_return(())
+
+    (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
+        dry_run=False,
+        repository_path='repo',
+        config={
+            'source_directories': [],
+            'repositories': ['repo'],
+            'read_special': False,
+        },
+        patterns=[],
+        local_borg_version='1.2.3',
+        global_arguments=flexmock(log_json=False),
+        borgmatic_runtime_directory='/run/borgmatic',
+        stream_processes=flexmock(),
+    )
+
+    assert create_flags == ('borg', 'create', '--read-special', '--patterns-from', 'patterns')
+    assert create_positional_arguments == REPO_ARCHIVE
+    assert pattern_file
+
+
+def test_make_base_create_command_with_stream_processes_and_read_special_true_skips_special_files_excludes():
     flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
     flexmock(module).should_receive('write_patterns_file').and_return(None)
     flexmock(module).should_receive('make_list_filter_flags').and_return('FOO')