Fix unit/integration tests and add coverage (#1043).

borgmatic/hooks/data_source/btrfs.py

@@ -22,26 +22,34 @@ def use_streaming(hook_config, config):  # pragma: no cover
     return False
 
 
-def list_subvolume_paths(btrfs_command, subvolume_path):
+def get_contained_subvolume_paths(btrfs_command, subvolume_path):
     '''
-    Given the path of a Btrfs subvolume, return it in a sequence with the paths of its contained
-    subvolumes.
+    Given the path of a Btrfs subvolume, return it in a sequence along with the paths of its
+    contained subvolumes.
 
-    If the Btrfs command errors, log that error and return an empty sequence.
+    If the btrfs command errors, log that error and return an empty sequence.
     '''
     try:
         btrfs_output = borgmatic.execute.execute_command_and_capture_output(
-            tuple(btrfs_command.split(' ')) + ('subvolume', 'list', subvolume_path,),
+            tuple(btrfs_command.split(' '))
+            + (
+                'subvolume',
+                'list',
+                subvolume_path,
+            ),
             close_fds=True,
         )
     except subprocess.CalledProcessError as error:
-        logger.debug(error)
+        logger.debug(
+            f'Ignoring Btrfs subvolume {subvolume_path} because of error listing its subvolumes: {error}'
+        )
 
         return ()
 
     return (subvolume_path,) + tuple(
         os.path.join(subvolume_path, line.split(' ')[-1])
         for line in btrfs_output.splitlines()
+        if line.strip()
     )
 
 
@@ -74,9 +82,11 @@ def get_all_subvolume_paths(btrfs_command, findmnt_command):
                     # a subvolume might exist at /mnt/subvolume but be mounted at /home/myuser.
                     # findmnt is still useful though because it's a global way to discover all
                     # Btrfs subvolumes—even if we have to do some additional legwork ourselves.
-                    list_subvolume_paths(btrfs_command, filesystem['target']) if
-                    FINDMNT_BTRFS_ROOT_SUBVOLUME_OPTION in filesystem['options'].split(',')
-                    else (filesystem['target'],)
+                    (
+                        get_contained_subvolume_paths(btrfs_command, filesystem['target'])
+                        if FINDMNT_BTRFS_ROOT_SUBVOLUME_OPTION in filesystem['options'].split(',')
+                        else (filesystem['target'],)
+                    )
                     for filesystem in json.loads(findmnt_output)['filesystems']
                 )
             )
@@ -91,6 +101,12 @@ Subvolume = collections.namedtuple('Subvolume', ('path', 'contained_patterns'),
 
 
 def get_subvolume_property(btrfs_command, subvolume_path, property_name):
+    '''
+    Given a btrfs command, a subvolume path, and a property name to lookup, return the value of the
+    corresponding property.
+
+    Raise subprocess.CalledProcessError if the btrfs command errors.
+    '''
     output = borgmatic.execute.execute_command_and_capture_output(
         tuple(btrfs_command.split(' '))
         + (
@@ -124,10 +140,15 @@ def omit_read_only_subvolume_paths(btrfs_command, subvolume_paths):
     retained_subvolume_paths = []
 
     for subvolume_path in subvolume_paths:
-        if get_subvolume_property(btrfs_command, subvolume_path, 'ro'):
-            logger.debug(f'Ignoring Btrfs subvolume {subvolume_path} because it is read-only')
-        else:
-            retained_subvolume_paths.append(subvolume_path)
+        try:
+            if get_subvolume_property(btrfs_command, subvolume_path, 'ro'):
+                logger.debug(f'Ignoring Btrfs subvolume {subvolume_path} because it is read-only')
+            else:
+                retained_subvolume_paths.append(subvolume_path)
+        except subprocess.CalledProcessError as error:
+            logger.debug(
+                f'Error determining read-only status of Btrfs subvolume {subvolume_path}: {error}'
+            )
 
     return tuple(retained_subvolume_paths)
 

test_requirements.txt

@@ -1,30 +1,30 @@
 appdirs==1.4.4
-apprise==1.8.0
-attrs==23.2.0
-black==24.4.2
-certifi==2024.7.4
+apprise==1.9.3
+attrs==25.3.0
+black==25.1.0
+certifi==2025.6.15
 chardet==5.2.0
-click==8.1.7
-codespell==2.2.6
-coverage==7.5.1
-flake8==7.0.0
+click==8.2.1
+codespell==2.4.1
+coverage==7.9.1
+flake8==7.3.0
 flake8-quotes==3.4.0
 flake8-use-fstring==1.4
 flake8-variables-names==0.0.6
 flexmock==0.12.1
-idna==3.7
-isort==5.13.2
-jsonschema==4.22.0
-Markdown==3.6
+idna==3.10
+isort==6.0.1
+jsonschema==4.24.0
+Markdown==3.8.2
 mccabe==0.7.0
-packaging==24.0
+packaging==25.0
 pathspec==0.12.1
-pluggy==1.5.0
+pluggy==1.6.0
 py==1.11.0
-pycodestyle==2.11.1
-pyflakes==3.2.0
-pytest==8.2.1
-pytest-cov==5.0.0
+pycodestyle==2.14.0
+pyflakes==3.4.0
+pytest==8.4.1
+pytest-cov==6.2.1
 PyYAML>5.0.0
 regex
 requests==2.32.4

tests/unit/hooks/data_source/test_btrfs.py

@@ -5,7 +5,31 @@ from borgmatic.borg.pattern import Pattern, Pattern_source, Pattern_style, Patte
 from borgmatic.hooks.data_source import btrfs as module
 
 
-def test_get_subvolume_mount_points_parses_findmnt_output():
+def test_get_contained_subvolume_paths_parses_btrfs_output():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).with_args(('btrfs', 'subvolume', 'list', '/mnt0'), close_fds=True).and_return(
+        'ID 256 gen 28 top level 5 path @sub\nID 258 gen 17 top level 5 path snap\n\n'
+    )
+
+    assert module.get_contained_subvolume_paths('btrfs', '/mnt0') == (
+        '/mnt0',
+        '/mnt0/@sub',
+        '/mnt0/snap',
+    )
+
+
+def test_get_contained_subvolume_paths_swallows_called_process_error():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).with_args(('btrfs', 'subvolume', 'list', '/mnt0'), close_fds=True).and_raise(
+        module.subprocess.CalledProcessError(1, 'btrfs')
+    )
+
+    assert module.get_contained_subvolume_paths('btrfs', '/mnt0') == ()
+
+
+def test_get_all_subvolume_paths_parses_findmnt_output():
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output'
     ).and_return(
@@ -22,31 +46,53 @@ def test_get_subvolume_mount_points_parses_findmnt_output():
                  "source": "/dev/loop0",
                  "fstype": "btrfs",
                  "options": "rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/"
+              },
+              {
+                 "target": "/mnt2",
+                 "source": "/dev/loop0",
+                 "fstype": "btrfs",
+                 "options": "rw,relatime,ssd,space_cache=v2,subvolid=256,subvol=/"
               }
            ]
         }
         '''
     )
+    flexmock(module).should_receive('get_contained_subvolume_paths').with_args(
+        'btrfs', '/mnt0'
+    ).and_return(('/mnt0',))
+    flexmock(module).should_receive('get_contained_subvolume_paths').with_args(
+        'btrfs', '/mnt1'
+    ).and_return(('/mnt1', '/mnt1/sub'))
+    flexmock(module).should_receive('get_contained_subvolume_paths').with_args(
+        'btrfs', '/mnt2'
+    ).never()
 
-    assert module.get_subvolume_mount_points('findmnt') == ('/mnt0', '/mnt1')
+    assert module.get_all_subvolume_paths('btrfs', 'findmnt') == (
+        '/mnt0',
+        '/mnt1',
+        '/mnt1/sub',
+        '/mnt2',
+    )
 
 
-def test_get_subvolume_mount_points_with_invalid_findmnt_json_errors():
+def test_get_all_subvolume_paths_with_invalid_findmnt_json_errors():
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output'
     ).and_return('{')
+    flexmock(module).should_receive('get_contained_subvolume_paths').never()
 
     with pytest.raises(ValueError):
-        module.get_subvolume_mount_points('findmnt')
+        module.get_all_subvolume_paths('btrfs', 'findmnt')
 
 
-def test_get_subvolume_mount_points_with_findmnt_json_missing_filesystems_errors():
+def test_get_all_subvolume_paths_with_findmnt_json_missing_filesystems_errors():
     flexmock(module.borgmatic.execute).should_receive(
         'execute_command_and_capture_output'
     ).and_return('{"wtf": "something is wrong here"}')
+    flexmock(module).should_receive('get_contained_subvolume_paths').never()
 
     with pytest.raises(ValueError):
-        module.get_subvolume_mount_points('findmnt')
+        module.get_all_subvolume_paths('btrfs', 'findmnt')
 
 
 def test_get_subvolume_property_with_invalid_btrfs_output_errors():
@@ -82,7 +128,7 @@ def test_get_subvolume_property_passes_through_general_value():
     assert module.get_subvolume_property('btrfs', '/foo', 'thing') == 'value'
 
 
-def test_omit_read_only_subvolume_mount_points_filters_out_read_only():
+def test_omit_read_only_subvolume_paths_filters_out_read_only_subvolumes():
     flexmock(module).should_receive('get_subvolume_property').with_args(
         'btrfs', '/foo', 'ro'
     ).and_return(False)
@@ -93,17 +139,29 @@ def test_omit_read_only_subvolume_mount_points_filters_out_read_only():
         'btrfs', '/baz', 'ro'
     ).and_return(False)
 
-    assert module.omit_read_only_subvolume_mount_points('btrfs', ('/foo', '/bar', '/baz')) == (
+    assert module.omit_read_only_subvolume_paths('btrfs', ('/foo', '/bar', '/baz')) == (
         '/foo',
         '/baz',
     )
 
 
+def test_omit_read_only_subvolume_paths_filters_out_erroring_subvolumes():
+    flexmock(module).should_receive('get_subvolume_property').with_args(
+        'btrfs', '/foo', 'ro'
+    ).and_raise(module.subprocess.CalledProcessError(1, 'btrfs'))
+    flexmock(module).should_receive('get_subvolume_property').with_args(
+        'btrfs', '/bar', 'ro'
+    ).and_return(True)
+    flexmock(module).should_receive('get_subvolume_property').with_args(
+        'btrfs', '/baz', 'ro'
+    ).and_return(False)
+
+    assert module.omit_read_only_subvolume_paths('btrfs', ('/foo', '/bar', '/baz')) == ('/baz',)
+
+
 def test_get_subvolumes_collects_subvolumes_matching_patterns():
-    flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2'))
-    flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return(
-        ('/mnt1', '/mnt2')
-    )
+    flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     contained_pattern = Pattern(
         '/mnt1',
@@ -128,10 +186,8 @@ def test_get_subvolumes_collects_subvolumes_matching_patterns():
 
 
 def test_get_subvolumes_skips_non_root_patterns():
-    flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2'))
-    flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return(
-        ('/mnt1', '/mnt2')
-    )
+    flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
         'get_contained_patterns'
@@ -162,10 +218,8 @@ def test_get_subvolumes_skips_non_root_patterns():
 
 
 def test_get_subvolumes_skips_non_config_patterns():
-    flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2'))
-    flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return(
-        ('/mnt1', '/mnt2')
-    )
+    flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
         'get_contained_patterns'
@@ -196,10 +250,8 @@ def test_get_subvolumes_skips_non_config_patterns():
 
 
 def test_get_subvolumes_without_patterns_collects_all_subvolumes():
-    flexmock(module).should_receive('get_subvolume_mount_points').and_return(('/mnt1', '/mnt2'))
-    flexmock(module).should_receive('omit_read_only_subvolume_mount_points').and_return(
-        ('/mnt1', '/mnt2')
-    )
+    flexmock(module).should_receive('get_all_subvolume_paths').and_return(('/mnt1', '/mnt2'))
+    flexmock(module).should_receive('omit_read_only_subvolume_paths').and_return(('/mnt1', '/mnt2'))
 
     flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
         'get_contained_patterns'