More automated tests (#303).

borgmatic/commands/arguments.py

@@ -484,7 +484,10 @@ def add_arguments_from_schema(arguments_group, schema, unparsed_arguments, names
 
     metavar = names[-1].upper()
     description = make_argument_description(schema, flag_name)
-    argument_type = borgmatic.config.schema.parse_type(schema_type)
+
+    # array=str instead of list here to support specifying a list as a YAML string on the
+    # command-line.
+    argument_type = borgmatic.config.schema.parse_type(schema_type, array=str)
     full_flag_name = f"--{flag_name.replace('_', '-')}"
 
     # As a UX nicety, allow boolean options that have a default of false to have command-line flags

borgmatic/config/arguments.py

@@ -3,6 +3,8 @@ import re
 
 import ruamel.yaml
 
+import borgmatic.config.schema
+
 LIST_INDEX_KEY_PATTERN = re.compile(r'^(?P<list_name>[a-zA-z-]+)\[(?P<index>\d+)\]$')
 
 
@@ -45,7 +47,7 @@ def set_values(config, keys, value):
         try:
             set_values(config[list_key][list_index], keys[1:], value)
         except IndexError:
-            raise ValueError(f'The list index {first_key} is out of range')
+            raise ValueError(f'Argument list index {first_key} is out of range')
 
         return
 
@@ -99,6 +101,7 @@ def convert_value_type(value, option_type):
     And if the source value isn't a string, return it as-is.
 
     Raise ruamel.yaml.error.YAMLError if there's a parse issue with the YAML.
+    Raise ValueError if the parsed value doesn't match the option type.
     '''
     if not isinstance(value, str):
         return value
@@ -106,7 +109,15 @@ def convert_value_type(value, option_type):
     if option_type == 'string':
         return value
 
-    return ruamel.yaml.YAML(typ='safe').load(io.StringIO(value))
+    try:
+        parsed_value = ruamel.yaml.YAML(typ='safe').load(io.StringIO(value))
+    except ruamel.yaml.error.YAMLError as error:
+        raise ValueError(f'Argument value "{value}" is invalid: {error.problem}')
+
+    if not isinstance(parsed_value, borgmatic.config.schema.parse_type(option_type)):
+        raise ValueError(f'Argument value "{value}" is not of the expected type: {option_type}')
+
+    return parsed_value
 
 
 def prepare_arguments_for_config(global_arguments, schema):
@@ -122,39 +133,34 @@ def prepare_arguments_for_config(global_arguments, schema):
 
         (
             (('my_option', 'sub_option'), 'value1'),
-            (('other_option'), 'value2'),
+            (('other_option',), 'value2'),
         )
-
-    Raise ValueError if an override can't be parsed.
     '''
     prepared_values = []
 
     for argument_name, value in global_arguments.__dict__.items():
-        try:
-            if value is None:
-                continue
-
-            keys = tuple(argument_name.split('.'))
-            option_type = type_for_option(schema, keys)
-
-            # The argument doesn't correspond to any option in the schema, so ignore it. It's
-            # probably a flag that borgmatic has on the command-line but not in configuration.
-            if option_type is None:
-                continue
-
-            prepared_values.append(
-                (
-                    keys,
-                    convert_value_type(value, option_type),
-                )
+        if value is None:
+            continue
+
+        keys = tuple(argument_name.split('.'))
+        option_type = type_for_option(schema, keys)
+
+        # The argument doesn't correspond to any option in the schema, so ignore it. It's
+        # probably a flag that borgmatic has on the command-line but not in configuration.
+        if option_type is None:
+            continue
+
+        prepared_values.append(
+            (
+                keys,
+                convert_value_type(value, option_type),
             )
-        except ruamel.yaml.error.YAMLError as error:
-            raise ValueError(f'Invalid override "{argument_name}": {error.problem}')
+        )
 
     return tuple(prepared_values)
 
 
-def apply_arguments_to_config(config, schema, global_arguments):
+def apply_arguments_to_config(config, schema, global_arguments):  # pragma: no cover
     '''
     Given a configuration dict, a corresponding configuration schema dict, and global arguments as
     an argparse.Namespace, set those given argument values into their corresponding configuration
@@ -164,6 +170,5 @@ def apply_arguments_to_config(config, schema, global_arguments):
     configuration object. Additionally, flags like "--foo.bar[0].baz" are supported to update a list
     element in the configuration.
     '''
-
     for keys, value in prepare_arguments_for_config(global_arguments, schema):
         set_values(config, keys, value)

borgmatic/config/schema.py

@@ -23,22 +23,27 @@ def get_properties(schema):
     return schema.get('properties', {})
 
 
-def parse_type(schema_type):
+def parse_type(schema_type, **overrides):
     '''
     Given a schema type as a string, return the corresponding Python type.
 
+    If any overrides are given in the from of a schema type string to a Python type, then override
+    the default type mapping with them.
+
     Raise ValueError if the schema type is unknown.
     '''
     try:
-        return {
-            'string': str,
-            'integer': int,
-            'number': decimal.Decimal,
-            'boolean': bool,
-            # This is str instead of list to support specifying a list as a YAML string on the
-            # command-line.
-            'array': str,
-        }[schema_type]
+        return dict(
+            {
+                'array': list,
+                'boolean': bool,
+                'integer': int,
+                'number': decimal.Decimal,
+                'object': dict,
+                'string': str,
+            },
+            **overrides,
+        )[schema_type]
     except KeyError:
         raise ValueError(f'Unknown type in configuration schema: {schema_type}')
 

tests/integration/commands/test_arguments.py

@@ -71,9 +71,9 @@ def test_add_arguments_from_schema_with_nested_object_adds_flag_for_each_option(
                     'properties': {
                         'bar': {'type': 'integer', 'description': 'help 1'},
                         'baz': {'type': 'string', 'description': 'help 2'},
-                    }
+                    },
                 }
-            }
+            },
         },
         unparsed_arguments=(),
     )
@@ -109,11 +109,11 @@ def test_add_arguments_from_schema_with_array_and_nested_object_adds_multiple_fl
                                 'type': 'integer',
                                 'description': 'help 1',
                             }
-                        }
+                        },
                     },
                     'description': 'help 2',
                 }
-            }
+            },
         },
         unparsed_arguments=(),
     )

tests/integration/config/test_arguments.py

@@ -0,0 +1,35 @@
+import pytest
+from flexmock import flexmock
+
+from borgmatic.config import arguments as module
+
+
+def test_convert_value_type_passes_through_non_string_value():
+    assert module.convert_value_type([1, 2], 'array') == [1, 2]
+
+
+def test_convert_value_type_passes_through_string_option_type():
+    assert module.convert_value_type('foo', 'string') == 'foo'
+
+
+def test_convert_value_type_parses_array_option_type():
+    assert module.convert_value_type('[foo, bar]', 'array') == ['foo', 'bar']
+
+
+def test_convert_value_type_with_array_option_type_and_no_array_raises():
+    with pytest.raises(ValueError):
+        module.convert_value_type('{foo, bar}', 'array')
+
+
+def test_convert_value_type_parses_object_option_type():
+    assert module.convert_value_type('{foo: bar}', 'object') == {'foo': 'bar'}
+
+
+def test_convert_value_type_with_invalid_value_raises():
+    with pytest.raises(ValueError):
+        module.convert_value_type('{foo, bar', 'object')
+
+
+def test_convert_value_type_with_unknown_option_type_raises():
+    with pytest.raises(ValueError):
+        module.convert_value_type('{foo, bar}', 'thingy')

tests/unit/commands/test_arguments.py

@@ -859,8 +859,12 @@ def test_add_arguments_from_schema_with_non_dict_schema_bails():
 
 def test_add_arguments_from_schema_with_nested_object_adds_flag_for_each_option():
     arguments_group = flexmock()
-    flexmock(module).should_receive('make_argument_description').and_return('help 1').and_return('help 2')
-    flexmock(module.borgmatic.config.schema).should_receive('parse_type').and_return(int).and_return(str)
+    flexmock(module).should_receive('make_argument_description').and_return('help 1').and_return(
+        'help 2'
+    )
+    flexmock(module.borgmatic.config.schema).should_receive('parse_type').and_return(
+        int
+    ).and_return(str)
     arguments_group.should_receive('add_argument').with_args(
         '--foo.bar',
         type=int,
@@ -885,9 +889,9 @@ def test_add_arguments_from_schema_with_nested_object_adds_flag_for_each_option(
                     'properties': {
                         'bar': {'type': 'integer'},
                         'baz': {'type': 'str'},
-                    }
+                    },
                 }
-            }
+            },
         },
         unparsed_arguments=(),
     )
@@ -914,9 +918,9 @@ def test_add_arguments_from_schema_uses_first_non_null_type_from_multi_type_obje
                     'type': ['null', 'object', 'boolean'],
                     'properties': {
                         'bar': {'type': 'integer'},
-                    }
+                    },
                 }
-            }
+            },
         },
         unparsed_arguments=(),
     )
@@ -939,9 +943,9 @@ def test_add_arguments_from_schema_with_empty_multi_type_raises():
                         'type': [],
                         'properties': {
                             'bar': {'type': 'integer'},
-                        }
+                        },
                     }
-                }
+                },
             },
             unparsed_arguments=(),
         )
@@ -962,7 +966,7 @@ def test_add_arguments_from_schema_with_propertyless_option_does_not_add_flag():
                 'foo': {
                     'type': 'object',
                 }
-            }
+            },
         },
         unparsed_arguments=(),
     )
@@ -989,9 +993,9 @@ def test_add_arguments_from_schema_with_array_adds_flag():
                     'type': 'array',
                     'items': {
                         'type': 'integer',
-                    }
+                    },
                 }
-            }
+            },
         },
         unparsed_arguments=(),
     )
@@ -999,8 +1003,12 @@ def test_add_arguments_from_schema_with_array_adds_flag():
 
 def test_add_arguments_from_schema_with_array_and_nested_object_adds_multiple_flags():
     arguments_group = flexmock()
-    flexmock(module).should_receive('make_argument_description').and_return('help 1').and_return('help 2')
-    flexmock(module.borgmatic.config.schema).should_receive('parse_type').and_return(int).and_return(str)
+    flexmock(module).should_receive('make_argument_description').and_return('help 1').and_return(
+        'help 2'
+    )
+    flexmock(module.borgmatic.config.schema).should_receive('parse_type').and_return(
+        int
+    ).and_return(str)
     arguments_group.should_receive('add_argument').with_args(
         '--foo[0].bar',
         type=int,
@@ -1033,10 +1041,10 @@ def test_add_arguments_from_schema_with_array_and_nested_object_adds_multiple_fl
                             'bar': {
                                 'type': 'integer',
                             }
-                        }
-                    }
+                        },
+                    },
                 }
-            }
+            },
         },
         unparsed_arguments=(),
     )
@@ -1063,7 +1071,7 @@ def test_add_arguments_from_schema_with_default_false_boolean_adds_valueless_fla
                     'type': 'boolean',
                     'default': False,
                 }
-            }
+            },
         },
         unparsed_arguments=(),
     )
@@ -1090,7 +1098,7 @@ def test_add_arguments_from_schema_with_default_true_boolean_adds_value_flag():
                     'type': 'boolean',
                     'default': True,
                 }
-            }
+            },
         },
         unparsed_arguments=(),
     )
@@ -1116,7 +1124,7 @@ def test_add_arguments_from_schema_with_defaultless_boolean_adds_value_flag():
                 'foo': {
                     'type': 'boolean',
                 }
-            }
+            },
         },
         unparsed_arguments=(),
     )
@@ -1151,7 +1159,7 @@ def test_add_arguments_from_schema_skips_omitted_flag_name():
                 'foo': {
                     'type': 'string',
                 },
-            }
+            },
         },
         unparsed_arguments=(),
     )
@@ -1177,7 +1185,7 @@ def test_add_arguments_from_schema_rewrites_option_name_to_flag_name():
                 'foo_and_stuff': {
                     'type': 'string',
                 },
-            }
+            },
         },
         unparsed_arguments=(),
     )

tests/unit/config/test_arguments.py

@@ -0,0 +1,195 @@
+import pytest
+from flexmock import flexmock
+
+from borgmatic.config import arguments as module
+
+
+def test_set_values_without_keys_bails():
+    config = {'option': 'value'}
+    module.set_values(config=config, keys=(), value=5)
+
+    assert config == {'option': 'value'}
+
+
+def test_set_values_with_keys_adds_them_to_config():
+    config = {'option': 'value'}
+
+    module.set_values(config=config, keys=('foo', 'bar', 'baz'), value=5)
+
+    assert config == {'option': 'value', 'foo': {'bar': {'baz': 5}}}
+
+
+def test_set_values_with_one_existing_key_adds_others_to_config():
+    config = {'foo': {'other': 'value'}}
+
+    module.set_values(config=config, keys=('foo', 'bar', 'baz'), value=5)
+
+    assert config == {'foo': {'other': 'value', 'bar': {'baz': 5}}}
+
+
+def test_set_values_with_two_existing_keys_adds_others_to_config():
+    config = {'foo': {'bar': {'other': 'value'}}}
+
+    module.set_values(config=config, keys=('foo', 'bar', 'baz'), value=5)
+
+    assert config == {'foo': {'bar': {'other': 'value', 'baz': 5}}}
+
+
+def test_set_values_with_list_index_key_adds_it_to_config():
+    config = {'foo': {'bar': [{'option': 'value'}, {'other': 'thing'}]}}
+
+    module.set_values(config=config, keys=('foo', 'bar[1]', 'baz'), value=5)
+
+    assert config == {'foo': {'bar': [{'option': 'value'}, {'other': 'thing', 'baz': 5}]}}
+
+
+def test_set_values_with_list_index_key_out_of_range_raises():
+    config = {'foo': {'bar': [{'option': 'value'}]}}
+
+    with pytest.raises(ValueError):
+        module.set_values(config=config, keys=('foo', 'bar[1]', 'baz'), value=5)
+
+
+def test_set_values_with_list_index_key_missing_list_and_out_of_range_raises():
+    config = {'other': 'value'}
+
+    with pytest.raises(ValueError):
+        module.set_values(config=config, keys=('foo', 'bar[1]', 'baz'), value=5)
+
+
+def test_set_values_with_final_list_index_key_adds_it_to_config():
+    config = {'foo': {'bar': [1, 2]}}
+
+    module.set_values(config=config, keys=('foo', 'bar[1]'), value=5)
+
+    assert config == {'foo': {'bar': [1, 5]}}
+
+
+def test_type_for_option_with_option_finds_type():
+    assert (
+        module.type_for_option(
+            schema={'type': 'object', 'properties': {'foo': {'type': 'integer'}}},
+            option_keys=('foo',),
+        )
+        == 'integer'
+    )
+
+
+def test_type_for_option_with_nested_option_finds_type():
+    assert (
+        module.type_for_option(
+            schema={
+                'type': 'object',
+                'properties': {
+                    'foo': {'type': 'object', 'properties': {'bar': {'type': 'boolean'}}}
+                },
+            },
+            option_keys=('foo', 'bar'),
+        )
+        == 'boolean'
+    )
+
+
+def test_type_for_option_with_missing_nested_option_finds_nothing():
+    assert (
+        module.type_for_option(
+            schema={
+                'type': 'object',
+                'properties': {
+                    'foo': {'type': 'object', 'properties': {'other': {'type': 'integer'}}}
+                },
+            },
+            option_keys=('foo', 'bar'),
+        )
+        is None
+    )
+
+
+def test_type_for_option_with_typeless_nested_option_finds_nothing():
+    assert (
+        module.type_for_option(
+            schema={
+                'type': 'object',
+                'properties': {'foo': {'type': 'object', 'properties': {'bar': {'example': 5}}}},
+            },
+            option_keys=('foo', 'bar'),
+        )
+        is None
+    )
+
+
+def test_type_for_list_index_option_finds_type():
+    assert (
+        module.type_for_option(
+            schema={
+                'type': 'object',
+                'properties': {'foo': {'type': 'array', 'items': {'type': 'integer'}}},
+            },
+            option_keys=('foo[0]',),
+        )
+        == 'integer'
+    )
+
+
+def test_type_for_nested_list_index_option_finds_type():
+    assert (
+        module.type_for_option(
+            schema={
+                'type': 'object',
+                'properties': {
+                    'foo': {
+                        'type': 'array',
+                        'items': {'type': 'object', 'properties': {'bar': {'type': 'integer'}}},
+                    }
+                },
+            },
+            option_keys=('foo[0]', 'bar'),
+        )
+        == 'integer'
+    )
+
+
+def test_prepare_arguments_for_config_converts_arguments_to_keys():
+    assert module.prepare_arguments_for_config(
+        global_arguments=flexmock(**{'my_option.sub_option': 'value1', 'other_option': 'value2'}),
+        schema={
+            'type': 'object',
+            'properties': {
+                'my_option': {'type': 'object', 'properties': {'sub_option': {'type': 'string'}}},
+                'other_option': {'type': 'string'},
+            },
+        },
+    ) == (
+        (('my_option', 'sub_option'), 'value1'),
+        (('other_option',), 'value2'),
+    )
+
+
+def test_prepare_arguments_for_config_skips_option_with_none_value():
+    assert module.prepare_arguments_for_config(
+        global_arguments=flexmock(**{'my_option.sub_option': None, 'other_option': 'value2'}),
+        schema={
+            'type': 'object',
+            'properties': {
+                'my_option': {'type': 'object', 'properties': {'sub_option': {'type': 'string'}}},
+                'other_option': {'type': 'string'},
+            },
+        },
+    ) == (
+        (('other_option',), 'value2'),
+    )
+
+
+def test_prepare_arguments_for_config_skips_option_missing_from_schema():
+    assert module.prepare_arguments_for_config(
+        global_arguments=flexmock(**{'my_option.sub_option': 'value1', 'other_option': 'value2'}),
+        schema={
+            'type': 'object',
+            'properties': {
+                'my_option': {'type': 'object'},
+                'other_option': {'type': 'string'},
+            },
+        },
+    ) == (
+        (('other_option',), 'value2'),
+    )