Add tests for adding array element arguments and fix the code under test (#303).

borgmatic/commands/arguments.py

@@ -299,31 +299,32 @@ def make_argument_description(schema, flag_name):
     '''
     description = schema.get('description')
     schema_type = schema.get('type')
+    example = schema.get('example')
 
     if not description:
         return None
 
-    if schema_type == 'array':
+    if '[0]' in flag_name:
+        description += ' To specify a different list element, replace the "[0]" with another array index ("[1]", "[2]", etc.).'
+
+    if example and schema_type == 'array':
         example_buffer = io.StringIO()
         yaml = ruamel.yaml.YAML(typ='safe')
         yaml.default_flow_style = True
-        yaml.dump(schema.get('example'), example_buffer)
+        yaml.dump(example, example_buffer)
 
         description += f' Example value: "{example_buffer.getvalue().strip()}"'
 
-    if '[0]' in flag_name:
-        description += ' To specify a different list element, replace the "[0]" with another array index ("[1]", "[2]", etc.).'
-
     description = description.replace('%', '%%')
 
     return description
 
 
-def add_array_element_arguments_from_schema(arguments_group, schema, unparsed_arguments, flag_name):
+def add_array_element_arguments(arguments_group, unparsed_arguments, flag_name):
     r'''
-    Given an argparse._ArgumentGroup instance, a configuration schema dict, a sequence of unparsed
-    argument strings, and a dotted flag name, convert the schema into corresponding command-line
-    array element flags that correspond to the given unparsed arguments.
+    Given an argparse._ArgumentGroup instance, a sequence of unparsed argument strings, and a dotted
+    flag name, convert the schema into corresponding command-line array element flags that
+    correspond to the given unparsed arguments.
 
     Here's the background. We want to support flags that can have arbitrary indices like:
 
@@ -356,26 +357,39 @@ def add_array_element_arguments_from_schema(arguments_group, schema, unparsed_ar
     argument will match it when parsing is performed! In this manner, we're using the actual user
     CLI input to inform what exact flags we support!
     '''
-    if '[0]' not in flag_name or '--help' in unparsed_arguments:
+    if '[0]' not in flag_name or not unparsed_arguments or '--help' in unparsed_arguments:
         return
 
     pattern = re.compile(fr'^--{flag_name.replace("[0]", r"\[\d+\]").replace(".", r"\.")}$')
-    existing_flags = set(
-        itertools.chain(
-            *(group_action.option_strings for group_action in arguments_group._group_actions)
+
+    # Find an existing list index flag (and its action) corresponding to the given flag name. If one
+    # isn't found, bail.
+    try:
+        (argument_action, existing_flag_name) = next(
+            (action, action_flag_name)
+            for action in arguments_group._group_actions
+            for action_flag_name in action.option_strings
+            if pattern.match(action_flag_name)
+            if f'--{flag_name}'.startswith(action_flag_name)
         )
-    )
+    except StopIteration:
+        return
 
     for unparsed in unparsed_arguments:
         unparsed_flag_name = unparsed.split('=', 1)[0]
 
-        if pattern.match(unparsed_flag_name) and unparsed_flag_name not in existing_flags:
-            arguments_group.add_argument(
-                unparsed_flag_name,
-                type=argument_type,
-                metavar=metavar,
-                help=description,
-            )
+        if not pattern.match(unparsed_flag_name) or unparsed_flag_name == existing_flag_name:
+            continue
+
+        arguments_group.add_argument(
+            unparsed_flag_name,
+            choices=argument_action.choices,
+            default=argument_action.default,
+            dest=unparsed_flag_name.lstrip('-'),
+            nargs=argument_action.nargs,
+            required=argument_action.nargs,
+            type=argument_action.type,
+        )
 
 
 def add_arguments_from_schema(arguments_group, schema, unparsed_arguments, names=None):
@@ -482,7 +496,7 @@ def add_arguments_from_schema(arguments_group, schema, unparsed_arguments, names
             help=description,
         )
 
-    add_array_element_arguments_from_schema(arguments_group, schema, unparsed_arguments, flag_name)
+    add_array_element_arguments(arguments_group, unparsed_arguments, flag_name)
 
 
 def make_parsers(schema, unparsed_arguments):

tests/integration/commands/test_arguments.py

@@ -4,6 +4,42 @@ from flexmock import flexmock
 from borgmatic.commands import arguments as module
 
 
+def test_make_argument_description_with_array_adds_example():
+    assert module.make_argument_description(
+        schema={
+            'description': 'Thing.',
+            'type': 'array',
+            'example': [1, '- foo', {'bar': 'baz'}],
+        },
+        flag_name='flag',
+    ) == 'Thing. Example value: "[1, \'- foo\', bar: baz]"'
+
+
+def test_add_array_element_arguments_adds_arguments_for_array_index_flags():
+    parser = module.ArgumentParser(allow_abbrev=False, add_help=False)
+    arguments_group = parser.add_argument_group('arguments')
+    arguments_group.add_argument(
+        '--foo[0].val',
+        dest='--foo[0].val',
+    )
+
+    flexmock(arguments_group).should_receive('add_argument').with_args(
+        '--foo[25].val',
+        choices=object,
+        default=object,
+        dest='foo[25].val',
+        nargs=object,
+        required=object,
+        type=object,
+    ).once()
+
+    module.add_array_element_arguments(
+        arguments_group=arguments_group,
+        unparsed_arguments=('--foo[25].val', 'fooval', '--bar[1].val', 'barval'),
+        flag_name='foo[0].val',
+    )
+
+
 def test_parse_arguments_with_no_arguments_uses_defaults():
     config_paths = ['default']
     flexmock(module.collect).should_receive('get_default_config_paths').and_return(config_paths)

tests/unit/actions/test_repo_create.py

@@ -1,9 +1,10 @@
+import pytest
 from flexmock import flexmock
 
 from borgmatic.actions import repo_create as module
 
 
-def test_run_repo_create_does_not_raise():
+def test_run_repo_create_with_encryption_mode_argument_does_not_raise():
     flexmock(module.logger).answer = lambda message: None
     flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True)
     flexmock(module.borgmatic.borg.repo_create).should_receive('create_repository')
@@ -28,6 +29,57 @@ def test_run_repo_create_does_not_raise():
     )
 
 
+def test_run_repo_create_with_encryption_mode_option_does_not_raise():
+    flexmock(module.logger).answer = lambda message: None
+    flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True)
+    flexmock(module.borgmatic.borg.repo_create).should_receive('create_repository')
+    arguments = flexmock(
+        encryption_mode=None,
+        source_repository=flexmock(),
+        repository=flexmock(),
+        copy_crypt_key=flexmock(),
+        append_only=flexmock(),
+        storage_quota=flexmock(),
+        make_parent_dirs=flexmock(),
+    )
+
+    module.run_repo_create(
+        repository={'path': 'repo', 'encryption': flexmock()},
+        config={},
+        local_borg_version=None,
+        repo_create_arguments=arguments,
+        global_arguments=flexmock(dry_run=False),
+        local_path=None,
+        remote_path=None,
+    )
+
+
+def test_run_repo_create_without_encryption_mode_raises():
+    flexmock(module.logger).answer = lambda message: None
+    flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True)
+    flexmock(module.borgmatic.borg.repo_create).should_receive('create_repository')
+    arguments = flexmock(
+        encryption_mode=None,
+        source_repository=flexmock(),
+        repository=flexmock(),
+        copy_crypt_key=flexmock(),
+        append_only=flexmock(),
+        storage_quota=flexmock(),
+        make_parent_dirs=flexmock(),
+    )
+
+    with pytest.raises(ValueError):
+        module.run_repo_create(
+            repository={'path': 'repo'},
+            config={},
+            local_borg_version=None,
+            repo_create_arguments=arguments,
+            global_arguments=flexmock(dry_run=False),
+            local_path=None,
+            remote_path=None,
+        )
+
+
 def test_run_repo_create_bails_if_repository_does_not_match():
     flexmock(module.logger).answer = lambda message: None
     flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(

tests/unit/commands/test_arguments.py

@@ -575,3 +575,209 @@ def test_parse_arguments_for_actions_raises_error_when_no_action_is_specified():
 
     with pytest.raises(ValueError):
         module.parse_arguments_for_actions(('config',), action_parsers, global_parser)
+
+
+def test_make_argument_description_without_description_bails():
+    assert module.make_argument_description(
+        schema={
+            'description': None,
+            'type': 'not yours',
+        },
+        flag_name='flag',
+    ) is None
+
+
+def test_make_argument_description_with_array_adds_example():
+    buffer = flexmock()
+    buffer.should_receive('getvalue').and_return('[example]')
+    flexmock(module.io).should_receive('StringIO').and_return(buffer)
+    yaml = flexmock()
+    yaml.should_receive('dump')
+    flexmock(module.ruamel.yaml).should_receive('YAML').and_return(yaml)
+
+    assert module.make_argument_description(
+        schema={
+            'description': 'Thing.',
+            'type': 'array',
+            'example': ['example'],
+        },
+        flag_name='flag',
+    ) == 'Thing. Example value: "[example]"'
+
+
+def test_make_argument_description_with_array_skips_missing_example():
+    yaml = flexmock()
+    yaml.should_receive('dump').and_return('[example]')
+    flexmock(module.ruamel.yaml).should_receive('YAML').and_return(yaml)
+
+    assert module.make_argument_description(
+        schema={
+            'description': 'Thing.',
+            'type': 'array',
+        },
+        flag_name='flag',
+    ) == 'Thing.'
+
+
+def test_make_argument_description_with_array_index_in_flag_name_adds_to_description():
+    assert 'list element' in module.make_argument_description(
+        schema={
+            'description': 'Thing.',
+            'type': 'something',
+        },
+        flag_name='flag[0]',
+    )
+
+
+def test_make_argument_description_escapes_percent_character():
+    assert module.make_argument_description(
+        schema={
+            'description': '% Thing.',
+            'type': 'something',
+        },
+        flag_name='flag',
+    ) == '%% Thing.'
+
+
+def test_add_array_element_arguments_without_array_index_bails():
+    arguments_group = flexmock()
+    arguments_group.should_receive('add_argument').never()
+
+    module.add_array_element_arguments(
+        arguments_group=arguments_group,
+        unparsed_arguments=(),
+        flag_name='foo',
+    )
+
+
+def test_add_array_element_arguments_with_help_flag_bails():
+    arguments_group = flexmock()
+    arguments_group.should_receive('add_argument').never()
+
+    module.add_array_element_arguments(
+        arguments_group=arguments_group,
+        unparsed_arguments=('--foo', '--help', '--bar'),
+        flag_name='foo[0]',
+    )
+
+
+def test_add_array_element_arguments_without_any_flags_bails():
+    arguments_group = flexmock()
+    arguments_group.should_receive('add_argument').never()
+
+    module.add_array_element_arguments(
+        arguments_group=arguments_group,
+        unparsed_arguments=(),
+        flag_name='foo[0]',
+    )
+
+
+def test_add_array_element_arguments_without_array_index_flags_bails():
+    arguments_group = flexmock(
+        _group_actions=(
+            flexmock(
+                option_strings=('--foo[0].val',),
+            ),
+        )
+    )
+    arguments_group.should_receive('add_argument').never()
+
+    module.add_array_element_arguments(
+        arguments_group=arguments_group,
+        unparsed_arguments=('--foo', '--bar'),
+        flag_name='foo[0].val',
+    )
+
+
+def test_add_array_element_arguments_with_non_matching_array_index_flags_bails():
+    arguments_group = flexmock(
+        _group_actions=(
+            flexmock(
+                option_strings=('--foo[0].val',),
+            ),
+        )
+    )
+    arguments_group.should_receive('add_argument').never()
+
+    module.add_array_element_arguments(
+        arguments_group=arguments_group,
+        unparsed_arguments=('--foo', '--bar[25].val', 'barval'),
+        flag_name='foo[0].val',
+    )
+
+
+def test_add_array_element_arguments_with_identical_array_index_flag_bails():
+    arguments_group = flexmock(
+        _group_actions=(
+            flexmock(
+                option_strings=('--foo[0].val',),
+            ),
+        )
+    )
+    arguments_group.should_receive('add_argument').never()
+
+    module.add_array_element_arguments(
+        arguments_group=arguments_group,
+        unparsed_arguments=('--foo[0].val', 'fooval', '--bar'),
+        flag_name='foo[0].val',
+    )
+
+
+def test_add_array_element_arguments_adds_arguments_for_array_index_flags():
+    arguments_group = flexmock(
+        _group_actions=(
+            flexmock(
+                option_strings=('--foo[0].val',),
+                choices=flexmock(),
+                default=flexmock(),
+                nargs=flexmock(),
+                required=flexmock(),
+                type=flexmock(),
+            ),
+        )
+    )
+    arguments_group.should_receive('add_argument').with_args(
+        '--foo[25].val',
+        choices=object,
+        default=object,
+        dest='foo[25].val',
+        nargs=object,
+        required=object,
+        type=object,
+    ).once()
+
+    module.add_array_element_arguments(
+        arguments_group=arguments_group,
+        unparsed_arguments=('--foo[25].val', 'fooval', '--bar[1].val', 'barval'),
+        flag_name='foo[0].val',
+    )
+
+
+def test_add_array_element_arguments_adds_arguments_for_array_index_flags_with_equals_sign():
+    arguments_group = flexmock(
+        _group_actions=(
+            flexmock(
+                option_strings=('--foo[0].val',),
+                choices=flexmock(),
+                default=flexmock(),
+                nargs=flexmock(),
+                required=flexmock(),
+                type=flexmock(),
+            ),
+        )
+    )
+    arguments_group.should_receive('add_argument').with_args(
+        '--foo[25].val',
+        choices=object,
+        default=object,
+        dest='foo[25].val',
+        nargs=object,
+        required=object,
+        type=object,
+    ).once()
+
+    module.add_array_element_arguments(
+        arguments_group=arguments_group,
+        unparsed_arguments=('--foo[25].val=fooval', '--bar[1].val=barval'),
+        flag_name='foo[0].val',
+    )