Remove click runners from test_config

tests/cli/test_config.py

@@ -6,8 +6,18 @@ import pytest
 from vdirsyncer import cli
 
 
-def test_read_config(monkeypatch):
+@pytest.fixture
-    f = io.StringIO(dedent(u'''
+def read_config(tmpdir):
+    def inner(cfg):
+        f = io.StringIO(dedent(cfg.format(base=str(tmpdir))))
+        return cli.utils.read_config(f)
+    return inner
+
+
+def test_read_config(read_config, monkeypatch):
+    errors = []
+    monkeypatch.setattr('vdirsyncer.cli.cli_logger.error', errors.append)
+    general, pairs, storages = read_config(u'''
         [general]
         status_path = /tmp/status/
 
@@ -29,11 +39,8 @@ def test_read_config(monkeypatch):
 
         [bogus]
         lol = true
-        '''))
+        ''')
 
-    errors = []
-    monkeypatch.setattr('vdirsyncer.cli.cli_logger.error', errors.append)
-    general, pairs, storages = cli.utils.read_config(f)
     assert general == {'status_path': '/tmp/status/'}
     assert pairs == {'bob': ('bob_a', 'bob_b', {'bam': True, 'foo': 'bar'})}
     assert storages == {
@@ -75,49 +82,45 @@ def test_parse_pairs_args():
     ]
 
 
-def test_missing_general_section(tmpdir, runner):
+def test_missing_general_section(read_config):
-    runner.cfg.write(dedent('''
+    with pytest.raises(cli.CliError) as excinfo:
-    [pair my_pair]
+        rv = read_config(u'''
-    a = my_a
+            [pair my_pair]
-    b = my_b
+            a = my_a
+            b = my_b
 
-    [storage my_a]
+            [storage my_a]
-    type = filesystem
+            type = filesystem
-    path = {0}/path_a/
+            path = {base}/path_a/
-    fileext = .txt
+            fileext = .txt
 
-    [storage my_b]
+            [storage my_b]
-    type = filesystem
+            type = filesystem
-    path = {0}/path_b/
+            path = {base}/path_b/
-    fileext = .txt
+            fileext = .txt
-    ''').format(str(tmpdir)))
+            ''')
 
-    result = runner.invoke(['sync'])
+    assert 'Invalid general section.' in excinfo.value.msg
-    assert result.exception
-    assert result.output.startswith('critical:')
-    assert 'invalid general section' in result.output.lower()
 
 
-def test_wrong_general_section(tmpdir, runner):
+def test_wrong_general_section(read_config):
-    runner.cfg.write(dedent('''
+    with pytest.raises(cli.CliError) as excinfo:
-    [general]
+        rv = read_config(u'''
-    wrong = true
+            [general]
-    '''))
+            wrong = true
-    result = runner.invoke(['sync'])
+            ''')
 
-    assert result.exception
+    assert 'Invalid general section.' in excinfo.value.msg
-    lines = result.output.splitlines()
+    assert excinfo.value.problems == [
-    assert lines[:-2] == [
+        'general section doesn\'t take the parameters: wrong',
-        'critical: general section doesn\'t take the parameters: wrong',
+        'general section is missing the parameters: status_path'
-        'critical: general section is missing the parameters: status_path'
     ]
-    assert 'Invalid general section.' in lines[-2]
 
 
 def test_invalid_storage_name():
     f = io.StringIO(dedent(u'''
         [general]
-        status_path = /tmp/status/
+        status_path = {base}/status/
 
         [storage foo.bar]
         '''))
@@ -156,3 +159,33 @@ def test_parse_config_value(capsys):
     assert x('3.14') == (3.14, 0)
     assert x('') == ('', 0)
     assert x('""') == ('', 0)
+
+
+def test_invalid_collections_arg():
+    f = io.StringIO(dedent(u'''
+        [general]
+        status_path = /tmp/status/
+
+        [pair foobar]
+        a = foo
+        b = bar
+        collections = [null]
+
+        [storage foo]
+        type = filesystem
+        path = /tmp/foo/
+        fileext = .txt
+
+        [storage bar]
+        type = filesystem
+        path = /tmp/bar/
+        fileext = .txt
+        '''))
+
+    with pytest.raises(cli.utils.CliError) as excinfo:
+        cli.utils.read_config(f)
+
+    assert (
+        'Section `pair foobar`: `collections` parameter must be a list of '
+        'collection names (strings!) or `null`.'
+    ) in str(excinfo.value)

tests/cli/test_main.py

@@ -227,32 +227,6 @@ def test_multiple_pairs(tmpdir, runner):
     ])
 
 
-def test_invalid_collections_arg(tmpdir, runner):
-    runner.write_with_general(dedent('''
-    [pair foobar]
-    a = foo
-    b = bar
-    collections = [null]
-
-    [storage foo]
-    type = filesystem
-    path = {base}/foo/
-    fileext = .txt
-
-    [storage bar]
-    type = filesystem
-    path = {base}/bar/
-    fileext = .txt
-    '''.format(base=str(tmpdir))))
-
-    result = runner.invoke(['sync'])
-    assert result.exception
-    assert result.output.strip().endswith(
-        'Section `pair foobar`: `collections` parameter must be a list of '
-        'collection names (strings!) or `null`.'
-    )
-
-
 def test_create_collections(tmpdir, runner):
     runner.write_with_general(dedent('''
     [pair foobar]

vdirsyncer/cli/utils.py

@@ -38,7 +38,21 @@ SECTION_NAME_CHARS = frozenset(chain(string.ascii_letters, string.digits, '_'))
 
 
 class CliError(RuntimeError):
-    pass
+    def __init__(self, msg, problems=None):
+        self.msg = msg
+        self.problems = problems
+        RuntimeError.__init__(self, msg)
+
+    def format_cli(self):
+        msg = self.msg.rstrip(u'.:')
+        if self.problems:
+            msg += u':'
+            if len(self.problems) == 1:
+                msg += u' {}'.format(self.problems[0])
+            else:
+                msg += u'\n' + u'\n  - '.join(self.problems) + u'\n\n'
+
+        return msg
 
 
 class JobFailed(RuntimeError):
@@ -56,7 +70,7 @@ def handle_cli_error(status_name=None):
     try:
         raise
     except CliError as e:
-        cli_logger.critical(str(e))
+        cli_logger.critical(e.format_cli())
     except StorageEmpty as e:
         cli_logger.error(
             '{status_name}: Storage "{name}" was completely emptied. Use '
@@ -247,19 +261,20 @@ def _validate_general_section(general_config):
 
     invalid = set(general_config) - GENERAL_ALL
     missing = GENERAL_REQUIRED - set(general_config)
+    problems = []
 
     if invalid:
-        cli_logger.critical(u'general section doesn\'t take the parameters: {}'
+        problems.append(u'general section doesn\'t take the parameters: {}'
-                            .format(u', '.join(invalid)))
+                        .format(u', '.join(invalid)))
 
     if missing:
-        cli_logger.critical(u'general section is missing the parameters: {}'
+        problems.append(u'general section is missing the parameters: {}'
-                            .format(u', '.join(missing)))
+                        .format(u', '.join(missing)))
 
-    if invalid or missing:
+    if problems:
-        raise ValueError('Invalid general section. You should copy the '
+        raise CliError(u'Invalid general section. You should copy the example '
-                         'example config from the repository and edit it.\n{}'
+                       u'config from the repository and edit it: {}\n'
-                         .format(PROJECT_HOME))
+                       .format(PROJECT_HOME), problems=problems)
 
 
 def _validate_pair_section(pair_config):
@@ -283,11 +298,6 @@ def load_config():
     try:
         with open(fname) as f:
             general, pairs, storages = read_config(f)
-        _validate_general_section(general)
-        general['status_path'] = os.path.join(
-            os.path.dirname(fname),
-            expand_path(general['status_path'])
-        )
     except Exception as e:
         raise CliError('Error during reading config {}: {}'
                        .format(fname, e))
@@ -339,6 +349,12 @@ def read_config(f):
         except ValueError as e:
             raise CliError('Section `{}`: {}'.format(section, str(e)))
 
+    _validate_general_section(general)
+    if getattr(f, 'name', None):
+        general['status_path'] = os.path.join(
+            os.path.dirname(f.name),
+            expand_path(general['status_path'])
+        )
     return general, pairs, storages
 
 
@@ -434,6 +450,8 @@ def handle_storage_init_error(cls, config):
     missing = required - given
     invalid = given - all
 
+    problems = []
+
     if missing:
         cli_logger.critical(
             u'{} storage requires the parameters: {}'
@@ -444,10 +462,12 @@ def handle_storage_init_error(cls, config):
             u'{} storage doesn\'t take the parameters: {}'
             .format(cls.storage_name, u', '.join(invalid)))
 
-    if not missing and not invalid:
+    if not problems:
         cli_logger.exception('')
+        problems.append('Internal error, see above.')
 
-    raise CliError('Failed to initialize {}.'.format(config['instance_name']))
+    raise CliError(u'Failed to initialize {}'.format(config['instance_name']),
+                   problems=problems)
 
 
 def parse_pairs_args(pairs_args, all_pairs):