From 93000698174366f737dd4979d79379471a3c1eb5 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sat, 7 Mar 2015 15:06:26 +0100 Subject: [PATCH] Remove click runners from test_config --- tests/cli/test_config.py | 107 +++++++++++++++++++++++++-------------- tests/cli/test_main.py | 26 ---------- vdirsyncer/cli/utils.py | 54 +++++++++++++------- 3 files changed, 107 insertions(+), 80 deletions(-) diff --git a/tests/cli/test_config.py b/tests/cli/test_config.py index fc9756d..ef143e2 100644 --- a/tests/cli/test_config.py +++ b/tests/cli/test_config.py @@ -6,8 +6,18 @@ import pytest from vdirsyncer import cli -def test_read_config(monkeypatch): - f = io.StringIO(dedent(u''' +@pytest.fixture +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): - runner.cfg.write(dedent(''' - [pair my_pair] - a = my_a - b = my_b +def test_missing_general_section(read_config): + with pytest.raises(cli.CliError) as excinfo: + rv = read_config(u''' + [pair my_pair] + a = my_a + b = my_b - [storage my_a] - type = filesystem - path = {0}/path_a/ - fileext = .txt + [storage my_a] + type = filesystem + path = {base}/path_a/ + fileext = .txt - [storage my_b] - type = filesystem - path = {0}/path_b/ - fileext = .txt - ''').format(str(tmpdir))) + [storage my_b] + type = filesystem + path = {base}/path_b/ + fileext = .txt + ''') - result = runner.invoke(['sync']) - assert result.exception - assert result.output.startswith('critical:') - assert 'invalid general section' in result.output.lower() + assert 'Invalid general section.' in excinfo.value.msg -def test_wrong_general_section(tmpdir, runner): - runner.cfg.write(dedent(''' - [general] - wrong = true - ''')) - result = runner.invoke(['sync']) +def test_wrong_general_section(read_config): + with pytest.raises(cli.CliError) as excinfo: + rv = read_config(u''' + [general] + wrong = true + ''') - assert result.exception - lines = result.output.splitlines() - assert lines[:-2] == [ - 'critical: general section doesn\'t take the parameters: wrong', - 'critical: general section is missing the parameters: status_path' + assert 'Invalid general section.' in excinfo.value.msg + assert excinfo.value.problems == [ + 'general section doesn\'t take the parameters: wrong', + '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) diff --git a/tests/cli/test_main.py b/tests/cli/test_main.py index eea5b56..58ab92e 100644 --- a/tests/cli/test_main.py +++ b/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] diff --git a/vdirsyncer/cli/utils.py b/vdirsyncer/cli/utils.py index 1a31e66..c2f726e 100644 --- a/vdirsyncer/cli/utils.py +++ b/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: {}' - .format(u', '.join(invalid))) + problems.append(u'general section doesn\'t take the parameters: {}' + .format(u', '.join(invalid))) if missing: - cli_logger.critical(u'general section is missing the parameters: {}' - .format(u', '.join(missing))) + problems.append(u'general section is missing the parameters: {}' + .format(u', '.join(missing))) - if invalid or missing: - raise ValueError('Invalid general section. You should copy the ' - 'example config from the repository and edit it.\n{}' - .format(PROJECT_HOME)) + if problems: + raise CliError(u'Invalid general section. You should copy the example ' + u'config from the repository and edit it: {}\n' + .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):