From 8b889e9ecd148511c9853505ec825237e118253c Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Sun, 15 Jun 2014 20:49:46 +0200 Subject: [PATCH 1/9] Port CLI to Click. After a while i finally gave in and ported the whole command line interface of vdirsyncer to Click. While it might seem like a huge dependency, i hope we can eliminate some helper functions in the process, and maybe are more motivated to write a more beautiful and intuitive interface due to all the niceties Click provides. --- setup.py | 2 +- vdirsyncer/cli.py | 56 +++++++++++++----------------------- vdirsyncer/utils/__init__.py | 21 ++++++-------- vdirsyncer/utils/compat.py | 2 -- 4 files changed, 29 insertions(+), 52 deletions(-) diff --git a/setup.py b/setup.py index 63f4161..50f0f31 100644 --- a/setup.py +++ b/setup.py @@ -38,7 +38,7 @@ setup( 'console_scripts': ['vdirsyncer = vdirsyncer.cli:main'] }, install_requires=[ - 'argvard>=0.3.0', + 'click', 'requests', 'lxml', 'icalendar>=3.6', diff --git a/vdirsyncer/cli.py b/vdirsyncer/cli.py index 578be7f..0d202b4 100644 --- a/vdirsyncer/cli.py +++ b/vdirsyncer/cli.py @@ -11,7 +11,7 @@ import json import os import sys -import argvard +import click from . import log from .storage import storage_names @@ -206,41 +206,29 @@ def parse_pairs_args(pairs_args, all_pairs): def _main(env, file_cfg): general, all_pairs, all_storages = file_cfg - app = argvard.Argvard() - @app.option('--verbosity verbosity') - def verbose_option(context, verbosity): + @click.group() + @click.option('--verbosity', '-v', default='INFO', + help='Either CRITICAL, ERROR, WARNING, INFO or DEBUG') + def app(verbosity): ''' - Basically Python logging levels. - - CRITICAL: Config errors, at most. - - ERROR: Normal errors, at most. - - WARNING: Problems of which vdirsyncer thinks that it can handle them - itself, but which might crash other clients. - - INFO: Normal output. - - DEBUG: Show e.g. HTTP traffic. Not supposed to be readable by the - normal user. - + vdirsyncer -- synchronize calendars and contacts ''' verbosity = verbosity.upper() x = getattr(log.logging, verbosity, None) if x is None: - raise ValueError(u'Invalid verbosity value: {}'.format(verbosity)) - log.set_level(x) + cli_logger.critical(u'Invalid verbosity value: {}' + .format(verbosity)) + sys.exit(1) + else: + log.set_level(x) - sync_command = argvard.Command() - - @sync_command.option('--force-delete status_name') - def force_delete(context, status_name): - '''Pretty please delete all my data.''' - context.setdefault('force_delete', set()).add(status_name) - - @sync_command.main('[pairs...]') - def sync_main(context, pairs=None): + @app.command() + @click.argument('pairs', nargs=-1) + @click.option('--force-delete', multiple=True, + help=('Disable data-loss protection for the given pairs. ' + 'Can be passed multiple times')) + def sync(pairs, force_delete): ''' Synchronize the given pairs. If no pairs are given, all will be synchronized. @@ -253,7 +241,7 @@ def _main(env, file_cfg): ''' actions = [] handled_collections = set() - force_delete = context.get('force_delete', set()) + force_delete = set(force_delete) for pair_name, _collection in parse_pairs_args(pairs, all_pairs): for collection in expand_collection(pair_name, _collection, all_pairs, all_storages): @@ -293,16 +281,12 @@ def _main(env, file_cfg): from multiprocessing import Pool p = Pool(processes=general.get('processes', 0) or len(actions)) if not all(p.map_async(_sync_collection, actions).get(10**9)): - raise CliError() - - app.register_command('sync', sync_command) + sys.exit(1) try: app() except CliError as e: - msg = str(e) - if msg: - cli_logger.critical(msg) + cli_logger.critical(str(e)) sys.exit(1) diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py index c9ff753..8ab360e 100644 --- a/vdirsyncer/utils/__init__.py +++ b/vdirsyncer/utils/__init__.py @@ -10,9 +10,10 @@ import os import requests +import click from .. import exceptions, log -from .compat import get_raw_input, iteritems, urlparse +from .compat import iteritems, urlparse logger = log.get(__name__) @@ -155,8 +156,6 @@ def get_password(username, resource): """ - import getpass - for func in (_password_from_netrc, _password_from_keyring): password = func(username, resource) if password is not None: @@ -164,18 +163,14 @@ def get_password(username, resource): .format(username, func.__doc__)) return password - prompt = ('Server password for {} at the resource {}: ' + prompt = ('Server password for {} at the resource {}' .format(username, resource)) - password = getpass.getpass(prompt=prompt) + password = click.prompt(prompt=prompt, hide_input=True) - if keyring is not None: - answer = None - while answer not in ['', 'y', 'n']: - prompt = 'Save this password in the keyring? [y/N] ' - answer = get_raw_input(prompt).lower() - if answer == 'y': - keyring.set_password(password_key_prefix + resource, - username, password) + if keyring is not None and \ + click.confirm('Save this password in the keyring?', default=False): + keyring.set_password(password_key_prefix + resource, + username, password) return password diff --git a/vdirsyncer/utils/compat.py b/vdirsyncer/utils/compat.py index cdc03f6..94dc822 100644 --- a/vdirsyncer/utils/compat.py +++ b/vdirsyncer/utils/compat.py @@ -20,7 +20,6 @@ if PY2: text_type = unicode # flake8: noqa iteritems = lambda x: x.iteritems() itervalues = lambda x: x.itervalues() - get_raw_input = raw_input else: import urllib.parse as urlparse urlquote_plus = urlparse.quote_plus @@ -28,4 +27,3 @@ else: text_type = str iteritems = lambda x: x.items() itervalues = lambda x: x.values() - get_raw_input = input From e9722ee63d4af48941766d2b6f8f21fef12c843e Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Mon, 16 Jun 2014 22:24:20 +0200 Subject: [PATCH 2/9] Add test for simple usage --- tests/test_cli.py | 32 ++++++++++++++++++++++++++++ vdirsyncer/cli.py | 53 ++++++++++++++++++++++++++++------------------- vdirsyncer/log.py | 14 ++++++++++++- 3 files changed, 77 insertions(+), 22 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 8c3a3d8..5953ca2 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -8,6 +8,8 @@ ''' from textwrap import dedent +from click.testing import CliRunner + import vdirsyncer.cli as cli @@ -124,3 +126,33 @@ def test_parse_pairs_args(): ('one', 'c'), ('eins', None) ] + + +def test_simple_run(tmpdir): + runner = CliRunner() + config_file = tmpdir.join('config') + config_file.write(dedent(''' + [general] + status_path = {0}/status/ + + [pair my_pair] + a = my_a + b = my_b + + [storage my_a] + type = filesystem + path = {0}/path_a/ + fileext = .txt + + [storage my_b] + type = filesystem + path = {0}/path_b/ + fileext = .txt + ''').format(str(tmpdir))) + + result = runner.invoke( + cli.app, ['sync'], + env={'VDIRSYNCER_CONFIG': str(config_file)} + ) + assert not result.exception + assert result.output.lower().strip() == 'syncing my_pair' diff --git a/vdirsyncer/cli.py b/vdirsyncer/cli.py index 0d202b4..8317542 100644 --- a/vdirsyncer/cli.py +++ b/vdirsyncer/cli.py @@ -169,14 +169,6 @@ def expand_collection(pair, collection, all_pairs, all_storages): return [collection] -def main(): - env = os.environ - - fname = expand_path(env.get('VDIRSYNCER_CONFIG', '~/.vdirsyncer/config')) - cfg = load_config(fname) - _main(env, cfg) - - def parse_pairs_args(pairs_args, all_pairs): if not pairs_args: pairs_args = list(all_pairs) @@ -204,13 +196,12 @@ def parse_pairs_args(pairs_args, all_pairs): yield pair, c -def _main(env, file_cfg): - general, all_pairs, all_storages = file_cfg - +def _create_app(): @click.group() @click.option('--verbosity', '-v', default='INFO', help='Either CRITICAL, ERROR, WARNING, INFO or DEBUG') - def app(verbosity): + @click.pass_context + def app(ctx, verbosity): ''' vdirsyncer -- synchronize calendars and contacts ''' @@ -223,12 +214,21 @@ def _main(env, file_cfg): else: log.set_level(x) + if ctx.obj is None: + ctx.obj = {} + + if 'config' not in ctx.obj: + fname = expand_path(os.environ.get('VDIRSYNCER_CONFIG', + '~/.vdirsyncer/config')) + ctx.obj['config'] = load_config(fname) + @app.command() @click.argument('pairs', nargs=-1) @click.option('--force-delete', multiple=True, help=('Disable data-loss protection for the given pairs. ' 'Can be passed multiple times')) - def sync(pairs, force_delete): + @click.pass_context + def sync(ctx, pairs, force_delete): ''' Synchronize the given pairs. If no pairs are given, all will be synchronized. @@ -239,6 +239,8 @@ def _main(env, file_cfg): `vdirsyncer sync bob/first_collection` will sync "first_collection" from the pair "bob". ''' + general, all_pairs, all_storages = ctx.obj['config'] + actions = [] handled_collections = set() force_delete = set(force_delete) @@ -275,19 +277,20 @@ def _main(env, file_cfg): if processes == 1: cli_logger.debug('Not using multiprocessing.') - map(_sync_collection, actions) + rv = (_sync_collection(x) for x in actions) else: cli_logger.debug('Using multiprocessing.') from multiprocessing import Pool p = Pool(processes=general.get('processes', 0) or len(actions)) - if not all(p.map_async(_sync_collection, actions).get(10**9)): - sys.exit(1) + rv = p.map_async(_sync_collection, actions).get(10**9) - try: - app() - except CliError as e: - cli_logger.critical(str(e)) - sys.exit(1) + if not all(rv): + sys.exit(1) + + return app + +app = _create_app() +del _create_app def _sync_collection(x): @@ -344,3 +347,11 @@ def sync_collection(config_a, config_b, pair_name, collection, pair_options, save_status(general['status_path'], status_name, status) return rv + + +def main(): + try: + app() + except CliError as e: + cli_logger.critical(str(e)) + sys.exit(1) diff --git a/vdirsyncer/log.py b/vdirsyncer/log.py index 86eb740..8640915 100644 --- a/vdirsyncer/log.py +++ b/vdirsyncer/log.py @@ -10,7 +10,19 @@ import logging import sys -stdout_handler = logging.StreamHandler(sys.stdout) +class StdHandler(logging.StreamHandler): + '''Required hack for supporting streams monkeypatched by click.''' + def __init__(self, name): + logging.Handler.__init__(self) + self._name = name + self.stream + + @property + def stream(self): + return getattr(sys, self._name) + + +stdout_handler = StdHandler('stdout') default_level = logging.INFO From 3cc9081f1cca93ce58b201fe27f9292731090093 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Tue, 17 Jun 2014 15:09:46 +0200 Subject: [PATCH 3/9] More beautiful output --- vdirsyncer/cli.py | 4 ++-- vdirsyncer/log.py | 36 +++++++++++++++++++++++++----------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/vdirsyncer/cli.py b/vdirsyncer/cli.py index 8317542..4557981 100644 --- a/vdirsyncer/cli.py +++ b/vdirsyncer/cli.py @@ -317,7 +317,7 @@ def sync_collection(config_a, config_b, pair_name, collection, pair_options, ) except StorageEmpty as e: rv = False - cli_logger.critical( + cli_logger.error( '{collection}: Storage "{side}" ({storage}) was completely ' 'emptied. Use "--force-delete {status_name}" to synchronize that ' 'emptyness to the other side, or delete the status by yourself to ' @@ -330,7 +330,7 @@ def sync_collection(config_a, config_b, pair_name, collection, pair_options, ) except SyncConflict as e: rv = False - cli_logger.critical( + cli_logger.error( '{collection}: One item changed on both sides. Resolve this ' 'conflict manually, or by setting the `conflict_resolution` ' 'parameter in your config file.\n' diff --git a/vdirsyncer/log.py b/vdirsyncer/log.py index 8640915..89dd2e9 100644 --- a/vdirsyncer/log.py +++ b/vdirsyncer/log.py @@ -7,22 +7,36 @@ :license: MIT, see LICENSE for more details. ''' import logging -import sys +import click -class StdHandler(logging.StreamHandler): - '''Required hack for supporting streams monkeypatched by click.''' - def __init__(self, name): - logging.Handler.__init__(self) - self._name = name - self.stream +class ColorFormatter(logging.Formatter): + colors = { + 'error': dict(fg='red'), + 'exception': dict(fg='red'), + 'critical': dict(fg='red'), + 'debug': dict(fg='blue'), + 'warning': dict(fg='yellow') + } - @property - def stream(self): - return getattr(sys, self._name) + def format(self, record): + if not record.exc_info: + level = record.levelname.lower() + if level in self.colors: + prefix = click.style('{}: '.format(level), + **self.colors[level]) + record.msg = prefix + record.msg + + return logging.Formatter.format(self, record) -stdout_handler = StdHandler('stdout') +class ClickStream(object): + def write(self, string): + click.echo(string.rstrip()) + + +stdout_handler = logging.StreamHandler(ClickStream()) +stdout_handler.formatter = ColorFormatter() default_level = logging.INFO From a300c8c222d23efd35f1347d84596ff5dbe6fb42 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 18 Jun 2014 19:59:51 +0200 Subject: [PATCH 4/9] Only add stdout handler when running as cli --- vdirsyncer/cli.py | 2 ++ vdirsyncer/log.py | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/vdirsyncer/cli.py b/vdirsyncer/cli.py index 4557981..8c762a1 100644 --- a/vdirsyncer/cli.py +++ b/vdirsyncer/cli.py @@ -205,6 +205,8 @@ def _create_app(): ''' vdirsyncer -- synchronize calendars and contacts ''' + log.add_handler(log.stdout_handler) + verbosity = verbosity.upper() x = getattr(log.logging, verbosity, None) if x is None: diff --git a/vdirsyncer/log.py b/vdirsyncer/log.py index 89dd2e9..42a2d38 100644 --- a/vdirsyncer/log.py +++ b/vdirsyncer/log.py @@ -40,10 +40,14 @@ stdout_handler.formatter = ColorFormatter() default_level = logging.INFO +def add_handler(handler): + for logger in loggers.values(): + logger.addHandler(handler) + + def create_logger(name): x = logging.getLogger(name) x.setLevel(default_level) - x.addHandler(stdout_handler) return x From 65a845c7ca4350da09666bf246de5fffc7327705 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 18 Jun 2014 20:00:06 +0200 Subject: [PATCH 5/9] Add one more test --- tests/test_cli.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 5953ca2..91a0e28 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -129,7 +129,6 @@ def test_parse_pairs_args(): def test_simple_run(tmpdir): - runner = CliRunner() config_file = tmpdir.join('config') config_file.write(dedent(''' [general] @@ -150,9 +149,37 @@ def test_simple_run(tmpdir): fileext = .txt ''').format(str(tmpdir))) + runner = CliRunner() result = runner.invoke( cli.app, ['sync'], env={'VDIRSYNCER_CONFIG': str(config_file)} ) assert not result.exception assert result.output.lower().strip() == 'syncing my_pair' + + +def test_missing_general_section(tmpdir): + config_file = tmpdir.join('config') + config_file.write(dedent(''' + [pair my_pair] + a = my_a + b = my_b + + [storage my_a] + type = filesystem + path = {0}/path_a/ + fileext = .txt + + [storage my_b] + type = filesystem + path = {0}/path_b/ + fileext = .txt + ''').format(str(tmpdir))) + + runner = CliRunner() + result = runner.invoke( + cli.app, ['sync'], + env={'VDIRSYNCER_CONFIG': str(config_file)} + ) + assert result.exception + assert 'unable to find general section' in result.output.lower() From a6e2f23bbcf74f7821549c3113fc1e8b7a0ff979 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 18 Jun 2014 20:17:05 +0200 Subject: [PATCH 6/9] Catch exceptions with decorator This allows us to validate the exact output in the tests --- tests/test_cli.py | 2 +- vdirsyncer/cli.py | 43 +++++++++++++++++++++++++------------------ 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 91a0e28..2983ba1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -182,4 +182,4 @@ def test_missing_general_section(tmpdir): env={'VDIRSYNCER_CONFIG': str(config_file)} ) assert result.exception - assert 'unable to find general section' in result.output.lower() + assert 'critical: unable to find general section' in result.output.lower() diff --git a/vdirsyncer/cli.py b/vdirsyncer/cli.py index 8c762a1..6e2d0f5 100644 --- a/vdirsyncer/cli.py +++ b/vdirsyncer/cli.py @@ -10,6 +10,7 @@ import json import os import sys +import functools import click @@ -197,24 +198,37 @@ def parse_pairs_args(pairs_args, all_pairs): def _create_app(): + def catch_errors(f): + @functools.wraps(f) + def inner(*a, **kw): + try: + f(*a, **kw) + except CliError as e: + cli_logger.critical(str(e)) + sys.exit(1) + + return inner + + def validate_verbosity(ctx, param, value): + x = getattr(log.logging, value.upper(), None) + if x is None: + raise click.BadParameter('Invalid verbosity value {}. Must be ' + 'CRITICAL, ERROR, WARNING, INFO or DEBUG' + .format(value)) + return x + @click.group() @click.option('--verbosity', '-v', default='INFO', + callback=validate_verbosity, help='Either CRITICAL, ERROR, WARNING, INFO or DEBUG') @click.pass_context + @catch_errors def app(ctx, verbosity): ''' vdirsyncer -- synchronize calendars and contacts ''' log.add_handler(log.stdout_handler) - - verbosity = verbosity.upper() - x = getattr(log.logging, verbosity, None) - if x is None: - cli_logger.critical(u'Invalid verbosity value: {}' - .format(verbosity)) - sys.exit(1) - else: - log.set_level(x) + log.set_level(verbosity) if ctx.obj is None: ctx.obj = {} @@ -230,6 +244,7 @@ def _create_app(): help=('Disable data-loss protection for the given pairs. ' 'Can be passed multiple times')) @click.pass_context + @catch_errors def sync(ctx, pairs, force_delete): ''' Synchronize the given pairs. If no pairs are given, all will be @@ -291,7 +306,7 @@ def _create_app(): return app -app = _create_app() +app = main = _create_app() del _create_app @@ -349,11 +364,3 @@ def sync_collection(config_a, config_b, pair_name, collection, pair_options, save_status(general['status_path'], status_name, status) return rv - - -def main(): - try: - app() - except CliError as e: - cli_logger.critical(str(e)) - sys.exit(1) From 28b649dec99c6d7b8ac1665a0ec4d020b187e37d Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 18 Jun 2014 20:22:21 +0200 Subject: [PATCH 7/9] Don't use click's newline echoing click.echo does two write calls, which get flushed immediately. Using this in multiprocessing might make output overlap. Using a single write-call apparently resolves that issue, although i don't know why. A proper solution would be to use locks, but e.g. multiprocessing's logger doesn't seem have nearly as much flexibility as others. For now it seems to be bugfree enough. --- vdirsyncer/log.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vdirsyncer/log.py b/vdirsyncer/log.py index 42a2d38..2ee0e87 100644 --- a/vdirsyncer/log.py +++ b/vdirsyncer/log.py @@ -32,7 +32,7 @@ class ColorFormatter(logging.Formatter): class ClickStream(object): def write(self, string): - click.echo(string.rstrip()) + click.echo(string, nl=False) stdout_handler = logging.StreamHandler(ClickStream()) From 289ac2cfac8626f8f0db22ccc5bb915d600fb90a Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Wed, 18 Jun 2014 21:32:48 +0200 Subject: [PATCH 8/9] More tests --- tests/test_cli.py | 27 ++++++++++++++++++++++----- vdirsyncer/utils/compat.py | 4 ++-- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 2983ba1..c4d1ecf 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -149,14 +149,15 @@ def test_simple_run(tmpdir): fileext = .txt ''').format(str(tmpdir))) - runner = CliRunner() - result = runner.invoke( - cli.app, ['sync'], - env={'VDIRSYNCER_CONFIG': str(config_file)} - ) + runner = CliRunner(env={'VDIRSYNCER_CONFIG': str(config_file)}) + result = runner.invoke(cli.app, ['sync']) assert not result.exception assert result.output.lower().strip() == 'syncing my_pair' + tmpdir.join('path_a/haha.txt').write('UID:haha') + result = runner.invoke(cli.app, ['sync']) + assert tmpdir.join('path_b/haha.txt').read() == 'UID:haha' + def test_missing_general_section(tmpdir): config_file = tmpdir.join('config') @@ -183,3 +184,19 @@ def test_missing_general_section(tmpdir): ) assert result.exception assert 'critical: unable to find general section' in result.output.lower() + + +def test_verbosity(tmpdir): + runner = CliRunner() + config_file = tmpdir.join('config') + config_file.write(dedent(''' + [general] + status_path = {0}/status/ + ''').format(str(tmpdir))) + + result = runner.invoke( + cli.app, ['--verbosity=HAHA', 'sync'], + env={'VDIRSYNCER_CONFIG': str(config_file)} + ) + assert result.exception + assert 'invalid verbosity value' diff --git a/vdirsyncer/utils/compat.py b/vdirsyncer/utils/compat.py index 94dc822..e088e2d 100644 --- a/vdirsyncer/utils/compat.py +++ b/vdirsyncer/utils/compat.py @@ -12,7 +12,7 @@ import sys PY2 = sys.version_info[0] == 2 -if PY2: +if PY2: # pragma: no cover import urlparse from urllib import \ quote_plus as urlquote_plus, \ @@ -20,7 +20,7 @@ if PY2: text_type = unicode # flake8: noqa iteritems = lambda x: x.iteritems() itervalues = lambda x: x.itervalues() -else: +else: # pragma: no cover import urllib.parse as urlparse urlquote_plus = urlparse.quote_plus urlunquote_plus = urlparse.unquote_plus From 2fb58d6bcd8ce83e5ed33cb9ac52028913ea3719 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Thu, 19 Jun 2014 18:06:34 +0200 Subject: [PATCH 9/9] Fix bug in keyring fetching code Leading to infinite loop when the password is simply not there. Also fixed some import styling again --- tests/__init__.py | 4 ++++ tests/utils/test_main.py | 41 +++++++++++++++++++++++++++++++++--- vdirsyncer/cli.py | 2 +- vdirsyncer/log.py | 1 + vdirsyncer/utils/__init__.py | 9 +++++--- 5 files changed, 50 insertions(+), 7 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 0bfc4c9..0bc5e4b 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -15,6 +15,10 @@ from vdirsyncer.utils.vobject import normalize_item as _normalize_item vdirsyncer.log.set_level(vdirsyncer.log.logging.DEBUG) +def blow_up(*a, **kw): + raise AssertionError('Did not expect to be called.') + + def normalize_item(item): if not isinstance(item, text_type): item = item.raw diff --git a/tests/utils/test_main.py b/tests/utils/test_main.py index ca2672e..aeea986 100644 --- a/tests/utils/test_main.py +++ b/tests/utils/test_main.py @@ -7,11 +7,13 @@ :license: MIT, see LICENSE for more details. ''' +import click +from click.testing import CliRunner import pytest import vdirsyncer.utils as utils from vdirsyncer.utils.vobject import split_collection -from .. import normalize_item, SIMPLE_TEMPLATE, BARE_EVENT_TEMPLATE +from .. import blow_up, normalize_item, SIMPLE_TEMPLATE, BARE_EVENT_TEMPLATE def test_parse_options(): @@ -58,7 +60,7 @@ def test_get_password_from_netrc(monkeypatch): return username, 'bogus', password monkeypatch.setattr('netrc.netrc', Netrc) - monkeypatch.setattr('getpass.getpass', None) + monkeypatch.setattr('getpass.getpass', blow_up) _password = utils.get_password(username, resource) assert _password == password @@ -101,13 +103,46 @@ def test_get_password_from_system_keyring(monkeypatch, resources_to_test): return None monkeypatch.setattr('netrc.netrc', Netrc) - monkeypatch.setattr('getpass.getpass', None) + monkeypatch.setattr('getpass.getpass', blow_up) _password = utils.get_password(username, resource) assert _password == password assert netrc_calls == [hostname] +def test_get_password_from_prompt(monkeypatch): + getpass_calls = [] + + class Netrc(object): + def authenticators(self, hostname): + return None + + class Keyring(object): + def get_password(self, *a, **kw): + return None + + monkeypatch.setattr('netrc.netrc', Netrc) + monkeypatch.setattr(utils, 'keyring', Keyring()) + + user = 'my_user' + resource = 'http://example.com' + + @click.command() + def fake_app(): + x = utils.get_password(user, resource) + click.echo('Password is {}'.format(x)) + + runner = CliRunner() + result = runner.invoke(fake_app, input='my_password\n\n') + assert not result.exception + assert result.output.splitlines() == [ + 'Server password for {} at the resource {}: '.format(user, resource), + 'Save this password in the keyring? [y/N]: ', + 'Password is my_password' + ] + + + def test_get_class_init_args(): class Foobar(object): def __init__(self, foo, bar, baz=None): diff --git a/vdirsyncer/cli.py b/vdirsyncer/cli.py index 6e2d0f5..8896767 100644 --- a/vdirsyncer/cli.py +++ b/vdirsyncer/cli.py @@ -7,10 +7,10 @@ :license: MIT, see LICENSE for more details. ''' +import functools import json import os import sys -import functools import click diff --git a/vdirsyncer/log.py b/vdirsyncer/log.py index 2ee0e87..0f9b30f 100644 --- a/vdirsyncer/log.py +++ b/vdirsyncer/log.py @@ -7,6 +7,7 @@ :license: MIT, see LICENSE for more details. ''' import logging + import click diff --git a/vdirsyncer/utils/__init__.py b/vdirsyncer/utils/__init__.py index 8ab360e..fe5b704 100644 --- a/vdirsyncer/utils/__init__.py +++ b/vdirsyncer/utils/__init__.py @@ -9,9 +9,10 @@ import os -import requests import click +import requests + from .. import exceptions, log from .compat import iteritems, urlparse @@ -117,7 +118,9 @@ def _password_from_keyring(username, resource): parsed = urlparse.urlsplit(key) path = parsed.path - if path.endswith('/'): + if not path: + return None + elif path.endswith('/'): path = path.rstrip('/') else: path = path.rsplit('/', 1)[0] + '/' @@ -165,7 +168,7 @@ def get_password(username, resource): prompt = ('Server password for {} at the resource {}' .format(username, resource)) - password = click.prompt(prompt=prompt, hide_input=True) + password = click.prompt(prompt, hide_input=True) if keyring is not None and \ click.confirm('Save this password in the keyring?', default=False):