CommonOptions: add unit test

src/borg/archiver.py

@@ -1940,7 +1940,7 @@ class Archiver:
                     else:
                         self.common_options.setdefault(suffix, set()).add(kwargs['dest'])
                     kwargs['dest'] += suffix
-                    if not provide_defaults and 'default' in kwargs:
+                    if not provide_defaults:
                         # Interpolate help now, in case the %(default)d (or so) is mentioned,
                         # to avoid producing incorrect help output.
                         # Assumption: Interpolated output can safely be interpolated again,
@@ -1978,7 +1978,10 @@ class Archiver:
                         # and un-clobber the args (for tidiness - you *cannot* use the suffixed
                         # names for other purposes, obviously).
                         setattr(args, map_to, value)
+                    try:
                         delattr(args, map_from)
+                    except AttributeError:
+                        pass
 
             # Options with an "append" action need some special treatment. Instead of
             # overriding values, all specified values are merged together.
@@ -1986,8 +1989,11 @@ class Archiver:
                 option_value = []
                 for suffix in self.suffix_precedence:
                     # Find values of this suffix, if any, and add them to the final list
-                    values = getattr(args, dest + suffix, [])
-                    option_value.extend(values)
+                    extend_from = dest + suffix
+                    if extend_from in args:
+                        values = getattr(args, extend_from)
+                        delattr(args, extend_from)
+                        option_value.extend(values)
                 setattr(args, dest, option_value)
 
     def build_parser(self):

src/borg/testsuite/archiver.py

@@ -1,3 +1,4 @@
+import argparse
 import errno
 import json
 import logging
@@ -2995,3 +2996,116 @@ class TestBuildFilter:
         assert not filter(Item(path='shallow/'))  # can this even happen? paths are normalized...
         assert filter(Item(path='deep enough/file'))
         assert filter(Item(path='something/dir/file'))
+
+
+class TestCommonOptions:
+    @staticmethod
+    def define_common_options(add_common_option):
+        add_common_option('-h', '--help', action='help', help='show this help message and exit')
+        add_common_option('--critical', dest='log_level', help='foo',
+                          action='store_const', const='critical', default='warning')
+        add_common_option('--error', dest='log_level', help='foo',
+                          action='store_const', const='error', default='warning')
+        add_common_option('--append', dest='append', help='foo',
+                          action='append', metavar='TOPIC', default=[])
+        add_common_option('-p', '--progress', dest='progress', action='store_true', help='foo')
+        add_common_option('--lock-wait', dest='lock_wait', type=int, metavar='N', default=1,
+                          help='(default: %(default)d).')
+        add_common_option('--no-files-cache', dest='no_files_cache', action='store_false', help='foo')
+
+    @pytest.fixture
+    def basic_parser(self):
+        parser = argparse.ArgumentParser(prog='test', description='test parser', add_help=False)
+        parser.common_options = Archiver.CommonOptions(self.define_common_options,
+                                                       suffix_precedence=('_level0', '_level1'))
+        return parser
+
+    @pytest.fixture
+    def subparsers(self, basic_parser):
+        return basic_parser.add_subparsers(title='required arguments', metavar='<command>')
+
+    @pytest.fixture
+    def parser(self, basic_parser):
+        basic_parser.common_options.add_common_group(basic_parser, '_level0', provide_defaults=True)
+        return basic_parser
+
+    @pytest.fixture
+    def common_parser(self, parser):
+        common_parser = argparse.ArgumentParser(add_help=False, prog='test')
+        parser.common_options.add_common_group(common_parser, '_level1')
+        return common_parser
+
+    @pytest.fixture
+    def parse_vars_from_line(self, parser, subparsers, common_parser):
+        subparser = subparsers.add_parser('subcommand', parents=[common_parser], add_help=False,
+                                          description='foo', epilog='bar', help='baz',
+                                          formatter_class=argparse.RawDescriptionHelpFormatter)
+        subparser.set_defaults(func=1234)
+        subparser.add_argument('--append-only', dest='append_only', action='store_true')
+
+        def parse_vars_from_line(*line):
+            print(line)
+            args = parser.parse_args(line)
+            parser.common_options.resolve(args)
+            return vars(args)
+
+        return parse_vars_from_line
+
+    def test_simple(self, parse_vars_from_line):
+        assert parse_vars_from_line('--error') == {
+            'no_files_cache': True,
+            'append': [],
+            'lock_wait': 1,
+            'log_level': 'error',
+            'progress': False
+        }
+
+        assert parse_vars_from_line('--error', 'subcommand', '--critical') == {
+            'no_files_cache': True,
+            'append': [],
+            'lock_wait': 1,
+            'log_level': 'critical',
+            'progress': False,
+            'append_only': False,
+            'func': 1234,
+        }
+
+        with pytest.raises(SystemExit):
+            parse_vars_from_line('--append-only', 'subcommand')
+
+        assert parse_vars_from_line('--append=foo', '--append', 'bar', 'subcommand', '--append', 'baz') == {
+            'no_files_cache': True,
+            'append': ['foo', 'bar', 'baz'],
+            'lock_wait': 1,
+            'log_level': 'warning',
+            'progress': False,
+            'append_only': False,
+            'func': 1234,
+        }
+
+    @pytest.mark.parametrize('position', ('before', 'after', 'both'))
+    @pytest.mark.parametrize('flag,args_key,args_value', (
+        ('-p', 'progress', True),
+        ('--lock-wait=3', 'lock_wait', 3),
+        ('--no-files-cache', 'no_files_cache', False),
+    ))
+    def test_flag_position_independence(self, parse_vars_from_line, position, flag, args_key, args_value):
+        line = []
+        if position in ('before', 'both'):
+            line.append(flag)
+        line.append('subcommand')
+        if position in ('after', 'both'):
+            line.append(flag)
+
+        result = {
+            'no_files_cache': True,
+            'append': [],
+            'lock_wait': 1,
+            'log_level': 'warning',
+            'progress': False,
+            'append_only': False,
+            'func': 1234,
+        }
+        result[args_key] = args_value
+
+        assert parse_vars_from_line(*line) == result