Add tests for the CLI #21
Conversation
Codecov Report
@@ Coverage Diff @@
## master #21 +/- ##
==========================================
+ Coverage 82.52% 88.14% +5.62%
==========================================
Files 10 10
Lines 744 751 +7
Branches 104 105 +1
==========================================
+ Hits 614 662 +48
+ Misses 108 61 -47
- Partials 22 28 +6
Continue to review full report at Codecov.
|
| @@ -87,7 +90,7 @@ def cli(conf): | |||
| @click.option('--exchange', help=_exchange_help) | |||
| @click.option('--amqp-url', help=_amqp_url_help) | |||
| def consume(amqp_url, exchange, queue_name, routing_key, callback, app_name): | |||
|
|
|||
| """Consume messages from an AMQP queue using a Python callback.""" | |||
There was a problem hiding this comment.
Might be nice to also document the params and their types, but this is still an improvement over what was there before so feel free to defer.
There was a problem hiding this comment.
I need to double check, but I believe click uses this docblock for user help, and it also renders the options help, so unfortunately it looks weird when you add the standard docblocks. I think I can add type hints to the click options, though.
| _log.error('Failed to parse {}: {}'.format(config_path, str(e))) | ||
| raise exceptions.ConfigurationException(e) | ||
| msg = 'Failed to parse {}: error at line {}, column {}'.format( | ||
| config_path, e.line, e.col) |
fedora_messaging/exceptions.py
Outdated
| @@ -8,6 +8,12 @@ class BaseException(Exception): | |||
| class ConfigurationException(BaseException): | |||
| """Raised when there's an invalid configuration setting""" | |||
|
|
|||
| def __init__(self, message): | |||
| self.message = message | |||
There was a problem hiding this comment.
Might be good to document the type of message.
| @@ -0,0 +1 @@ | |||
| publish_exchange = I forgot quotes here | |||
There was a problem hiding this comment.
heh, GitHub does not like this.
| import unittest | ||
|
|
||
| from click.testing import CliRunner | ||
| import mock |
There was a problem hiding this comment.
In Python 3 mock is a built-in, and we're almost Python3-y.
There was a problem hiding this comment.
It makes me sad every time 😿
| """Assert a bad configuration file is reported.""" | ||
| runner = CliRunner() | ||
| result = runner.invoke(cli.cli, ['--conf=' + BAD_CONF, 'consume']) | ||
| self.assertEqual(2, result.exit_code) |
There was a problem hiding this comment.
It might be good to check that the error message contains some expected text.
| """Assert a missing configuration file is reported.""" | ||
| runner = CliRunner() | ||
| result = runner.invoke(cli.cli, ['--conf=thispathdoesnotexist', 'consume']) | ||
| self.assertEqual(2, result.exit_code) |
If a configuration file is explicitly specified, report an error if the file doesn't exist. Additionally, handle a configuration exception during configuration loading. Signed-off-by: Jeremy Cline <jcline@redhat.com>
Signed-off-by: Jeremy Cline <jcline@redhat.com>
Signed-off-by: Jeremy Cline <jcline@redhat.com>
|
On 08/17/2018 06:36 AM, Jeremy Cline wrote:
I need to double check, but I believe click uses this docblock for user
help, and it also renders the options help, so unfortunately it looks
weird when you add the standard docblocks. I think I can add type hints
to the click options, though.
Ah yes, this is true. Clement came up with a workaround that works,
though is kinda unconventional, in Bodhi's CLI. He basically put two
docblocks on the function. The first one gets used by click to render
help text, and the second gets using by nothing but humans, with their
soft human brains. Here's an example:
https://github.com/fedora-infra/bodhi/blob/3.8.1/bodhi/client/__init__.py#L208-L221
|
|
Ah, neato. |
Also fix a small bug from exception changes in config.py