From 65620395fbdbb0c23766b4552c1ade6eb3322eaa Mon Sep 17 00:00:00 2001 From: Matt Bennett Date: Wed, 7 Oct 2015 11:11:33 +0100 Subject: [PATCH 1/9] require nameko>=2.2.0 to make use of pytest plugin add test coverage for partial and disabled config --- conftest.py | 83 ------------------------------------------- setup.py | 2 +- test_nameko_sentry.py | 30 ++++++++++++++++ 3 files changed, 31 insertions(+), 84 deletions(-) delete mode 100644 conftest.py diff --git a/conftest.py b/conftest.py deleted file mode 100644 index d3c6165..0000000 --- a/conftest.py +++ /dev/null @@ -1,83 +0,0 @@ -from __future__ import absolute_import - -# all imports are inline to make sure they happen after eventlet.monkey_patch -# which is called in pytest_load_initial_conftests (calling monkey_patch at -# import time breaks the pytest capturemanager) - -import pytest - - -def pytest_addoption(parser): - parser.addoption( - '--blocking-detection', - action='store_true', - dest='blocking_detection', - default=False, - help='turn on eventlet hub blocking detection') - - parser.addoption( - "--log-level", action="store", - default='DEBUG', - help=("The logging-level for the test run.")) - - parser.addoption( - "--amqp-uri", action="store", dest='AMQP_URI', - default='amqp://guest:guest@localhost:5672/nameko_test', - help=("The AMQP-URI to connect to rabbit with.")) - - parser.addoption( - "--rabbit-ctl-uri", action="store", dest='RABBIT_CTL_URI', - default='http://guest:guest@localhost:15672', - help=("The URI for rabbit's management API.")) - - -def pytest_load_initial_conftests(): - # make sure we monkey_patch before local conftests - import eventlet - eventlet.monkey_patch() - - -def pytest_configure(config): - import logging - import sys - - if config.option.blocking_detection: # pragma: no cover - from eventlet import debug - debug.hub_blocking_detection(True) - - log_level = config.getoption('log_level') - if log_level is not None: - log_level = getattr(logging, log_level) - logging.basicConfig(level=log_level, stream=sys.stderr) - - -@pytest.fixture -def ensure_cleanup_order(request): - """ Ensure ``rabbit_config`` is invoked early if it's used by any fixture - in ``request``. - """ - if "rabbit_config" in request.funcargnames: - request.getfuncargvalue("rabbit_config") - - -@pytest.yield_fixture -def container_factory(ensure_cleanup_order): - from nameko.containers import ServiceContainer - - all_containers = [] - - def make_container(service_cls, config, worker_ctx_cls=None): - container = ServiceContainer(service_cls, config, worker_ctx_cls) - all_containers.append(container) - return container - - yield make_container - - for c in all_containers: - try: - c.stop() - except: # pragma: no cover - pass - - - diff --git a/setup.py b/setup.py index 4581bd6..2a551e0 100644 --- a/setup.py +++ b/setup.py @@ -10,7 +10,7 @@ url='http://github.com/mattbennett/nameko-sentry', py_modules=['nameko_sentry'], install_requires=[ - "nameko>=2.0.0", + "nameko>=2.2.0", "raven>=3.0.0" ], extras_require={ diff --git a/test_nameko_sentry.py b/test_nameko_sentry.py index 334fadb..66027fa 100644 --- a/test_nameko_sentry.py +++ b/test_nameko_sentry.py @@ -63,12 +63,42 @@ def test_setup(reporter): # client config and DSN applied correctly assert reporter.client.site == "site name" assert reporter.client.get_public_dsn() == "//user@localhost:9000/1" + assert reporter.client.is_enabled() # transport set correctly transport = reporter.client.remote.get_transport() assert isinstance(transport, EventletHTTPTransport) +def test_setup_without_optional_config(config): + + del config['SENTRY']['CLIENT_CONFIG'] + container = Mock(config=config) + + reporter = SentryReporter().bind(container, "sentry") + reporter.setup() + + # DSN applied correctly + assert reporter.client.get_public_dsn() == "//user@localhost:9000/1" + assert reporter.client.is_enabled() + + # transport set correctly + transport = reporter.client.remote.get_transport() + assert isinstance(transport, EventletHTTPTransport) + + +def test_disabled(config): + config['SENTRY']['DSN'] = None + container = Mock(config=config) + + reporter = SentryReporter().bind(container, "sentry") + reporter.setup() + + # DSN applied correctly + assert reporter.client.get_public_dsn() == None + assert not reporter.client.is_enabled() + + def test_worker_result(reporter, worker_ctx): result = "OK!" From 48512a375b24a02f04b5ee1c713be05c1bb4ca21 Mon Sep 17 00:00:00 2001 From: Matt Bennett Date: Tue, 24 Nov 2015 17:17:12 +0800 Subject: [PATCH 2/9] checkpoint: using nameko-controlled greenthread --- nameko_sentry.py | 37 +++++++++++++-- test_nameko_sentry.py | 102 ++++++++++++++++++++++++++++++++---------- 2 files changed, 111 insertions(+), 28 deletions(-) diff --git a/nameko_sentry.py b/nameko_sentry.py index 06b6b87..f3abc0d 100644 --- a/nameko_sentry.py +++ b/nameko_sentry.py @@ -1,20 +1,50 @@ import logging +from eventlet.queue import Queue from nameko.extensions import DependencyProvider from raven import Client -from raven.transport.eventlet import EventletHTTPTransport class SentryReporter(DependencyProvider): """ Send exceptions generated by entrypoints to a sentry server. """ + _gt = None + queue = None + client = None + + def _run(self): + + while True: + item = self.queue.get() + if item is None: + break + + exc_info, message, extra, data = item + self.client.captureException( + exc_info, message=message, extra=extra, data=data) + + # these will remain in scope until the next iteration and + # can potentially be large, so delete to reclaim the memory now + del exc_info, message, extra, data, item + + def start(self): + self._gt = self.container.spawn_managed_thread( + self._run, protected=True) + + def stop(self): + self.queue.put(None) + + if self._gt is not None: + self._gt.wait() + def setup(self): sentry_config = self.container.config.get('SENTRY') dsn = sentry_config['DSN'] kwargs = sentry_config.get('CLIENT_CONFIG', {}) - self.client = Client(dsn, transport=EventletHTTPTransport, **kwargs) + self.queue = Queue() + self.client = Client(dsn, **kwargs) def worker_result(self, worker_ctx, result, exc_info): if exc_info is None: @@ -51,5 +81,4 @@ def worker_result(self, worker_ctx, result, exc_info): extra = {'exc': exc} - self.client.captureException( - exc_info, message=message, extra=extra, data=data) + self.queue.put((exc_info, message, extra, data)) diff --git a/test_nameko_sentry.py b/test_nameko_sentry.py index 66027fa..1d09dd5 100644 --- a/test_nameko_sentry.py +++ b/test_nameko_sentry.py @@ -1,15 +1,14 @@ import logging -from mock import Mock, patch, call import pytest - +from mock import Mock, call, patch from nameko.containers import WorkerContext from nameko.extensions import Entrypoint from nameko.testing.services import dummy, entrypoint_hook, entrypoint_waiter from nameko.testing.utils import get_extension -from raven.transport.eventlet import EventletHTTPTransport - from nameko_sentry import SentryReporter +from raven.transport.threaded import ThreadedHTTPTransport +from util import wait_for_call # TODO: new lib class CustomException(Exception): @@ -28,6 +27,21 @@ def config(): } +@pytest.fixture +def service_cls(): + + class Service(object): + name = "service" + + sentry = SentryReporter() + + @dummy + def broken(self): + raise CustomException("Error!") + + return Service + + @pytest.fixture def container(config): return Mock(config=config) @@ -67,7 +81,7 @@ def test_setup(reporter): # transport set correctly transport = reporter.client.remote.get_transport() - assert isinstance(transport, EventletHTTPTransport) + assert isinstance(transport, ThreadedHTTPTransport) def test_setup_without_optional_config(config): @@ -84,7 +98,7 @@ def test_setup_without_optional_config(config): # transport set correctly transport = reporter.client.remote.get_transport() - assert isinstance(transport, EventletHTTPTransport) + assert isinstance(transport, ThreadedHTTPTransport) def test_disabled(config): @@ -103,10 +117,9 @@ def test_worker_result(reporter, worker_ctx): result = "OK!" reporter.setup() - with patch.object(reporter, 'client') as client: - reporter.worker_result(worker_ctx, result, None) + reporter.worker_result(worker_ctx, result, None) - assert not client.captureException.called + assert reporter.queue.qsize() == 0 def test_worker_exception(reporter, worker_ctx): @@ -115,50 +128,91 @@ def test_worker_exception(reporter, worker_ctx): exc_info = (CustomException, exc, None) reporter.setup() - with patch.object(reporter, 'client') as client: - reporter.worker_result(worker_ctx, None, exc_info) + reporter.worker_result(worker_ctx, None, exc_info) # generate expected call args logger = "{}.{}".format( worker_ctx.service_name, worker_ctx.entrypoint.method_name) - message = "Unhandled exception in call {}: {} {!r}".format( + expected_message = "Unhandled exception in call {}: {} {!r}".format( worker_ctx.call_id, CustomException.__name__, str(exc) ) - extra = {'exc': exc} + expected_extra = {'exc': exc} if isinstance(exc, worker_ctx.entrypoint.expected_exceptions): loglevel = logging.WARNING else: loglevel = logging.ERROR - data = { + expected_data = { 'logger': logger, 'level': loglevel, - 'message': message, + 'message': expected_message, 'tags': { 'call_id': worker_ctx.call_id, 'parent_call_id': worker_ctx.immediate_parent_call_id } } - # verify call + assert reporter.queue.qsize() == 1 + + _, message, extra, data = reporter.queue.get() + assert message == expected_message + assert extra == expected_extra + assert data == expected_data + + +def test_run(reporter): + + exc = CustomException("Error!") + exc_info = (CustomException, exc, None) + + message = "message" + extra = "extra" + data = "data" + + reporter.setup() + + reporter.queue.put((exc_info, message, extra, data)) + reporter.queue.put(None) + + with patch.object(reporter, 'client') as client: + reporter._run() + assert client.captureException.call_args_list == [ call(exc_info, message=message, extra=extra, data=data) ] -def test_end_to_end(container_factory, config): +def test_start(container_factory, service_cls, config): - class Service(object): - name = "service" + container = container_factory(service_cls, config) + reporter = get_extension(container, SentryReporter) - sentry = SentryReporter() + reporter.setup() - @dummy - def broken(self): - raise CustomException("Error!") + with patch.object(reporter, 'queue') as queue: + with wait_for_call(queue, 'get') as get: + reporter.start() + + assert reporter._gt is not None + assert get.called # do you need to patch as well? + + +def test_stop(container_factory, service_cls, config): + + container = container_factory(service_cls, config) + reporter = get_extension(container, SentryReporter) + + reporter.setup() + reporter.start() + assert not reporter._gt.dead + reporter.stop() + assert reporter._gt.dead + + +def test_end_to_end(container_factory, service_cls, config): - container = container_factory(Service, config) + container = container_factory(service_cls, config) container.start() reporter = get_extension(container, SentryReporter) From 622d0719e9347d76027553a4dd8379b4a01b552c Mon Sep 17 00:00:00 2001 From: Matt Bennett Date: Thu, 3 Dec 2015 16:16:31 +0000 Subject: [PATCH 3/9] use nameko's wait_for_call helper --- test_nameko_sentry.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test_nameko_sentry.py b/test_nameko_sentry.py index 1d09dd5..3dbdfd3 100644 --- a/test_nameko_sentry.py +++ b/test_nameko_sentry.py @@ -5,10 +5,10 @@ from nameko.containers import WorkerContext from nameko.extensions import Entrypoint from nameko.testing.services import dummy, entrypoint_hook, entrypoint_waiter +from nameko.testing.utils import wait_for_call # TODO: new lib from nameko.testing.utils import get_extension from nameko_sentry import SentryReporter from raven.transport.threaded import ThreadedHTTPTransport -from util import wait_for_call # TODO: new lib class CustomException(Exception): @@ -191,11 +191,12 @@ def test_start(container_factory, service_cls, config): reporter.setup() with patch.object(reporter, 'queue') as queue: - with wait_for_call(queue, 'get') as get: - reporter.start() + reporter.start() + with wait_for_call(1, queue.get): + pass assert reporter._gt is not None - assert get.called # do you need to patch as well? + assert queue.get.called def test_stop(container_factory, service_cls, config): From e26adbbffd47895c1e9b7281e72285583c4fa74f Mon Sep 17 00:00:00 2001 From: Matt Bennett Date: Thu, 3 Dec 2015 16:32:21 +0000 Subject: [PATCH 4/9] also test behaviour when stopping if not started --- test_nameko_sentry.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test_nameko_sentry.py b/test_nameko_sentry.py index 3dbdfd3..45c8d6c 100644 --- a/test_nameko_sentry.py +++ b/test_nameko_sentry.py @@ -210,6 +210,19 @@ def test_stop(container_factory, service_cls, config): reporter.stop() assert reporter._gt.dead + # subsequent stop should be a no-op + reporter.stop() + + +def test_stop_not_started(container_factory, service_cls, config): + + container = container_factory(service_cls, config) + reporter = get_extension(container, SentryReporter) + + reporter.setup() + assert reporter._gt is None + reporter.stop() + def test_end_to_end(container_factory, service_cls, config): From 7262d84fa9438eb8c0ce01747ffc38ebf9b52a30 Mon Sep 17 00:00:00 2001 From: Matt Bennett Date: Thu, 3 Dec 2015 17:24:45 +0000 Subject: [PATCH 5/9] use modern dependencies --- setup.py | 11 ++++------- test_nameko_sentry.py | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/setup.py b/setup.py index 2a551e0..64fe2e4 100644 --- a/setup.py +++ b/setup.py @@ -15,13 +15,10 @@ ], extras_require={ 'dev': [ - "coverage==4.0a1", - "flake8==2.1.0", - "mccabe==0.3", - "pep8==1.6.1", - "pyflakes==0.8.1", - "pylint==1.0.0", - "pytest==2.4.2", + "coverage==4.0.3", + "flake8==2.5.0", + "pylint==1.5.1", + "pytest==2.8.3", ] }, dependency_links=[], diff --git a/test_nameko_sentry.py b/test_nameko_sentry.py index 45c8d6c..0f0f013 100644 --- a/test_nameko_sentry.py +++ b/test_nameko_sentry.py @@ -109,7 +109,7 @@ def test_disabled(config): reporter.setup() # DSN applied correctly - assert reporter.client.get_public_dsn() == None + assert reporter.client.get_public_dsn() is None assert not reporter.client.is_enabled() From 675edff6a8002550f97279f8c84e6eb2d4fc652c Mon Sep 17 00:00:00 2001 From: Matt Bennett Date: Mon, 7 Dec 2015 10:28:13 +0000 Subject: [PATCH 6/9] test that queue is created correctly in setup --- test_nameko_sentry.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test_nameko_sentry.py b/test_nameko_sentry.py index 0f0f013..33e8106 100644 --- a/test_nameko_sentry.py +++ b/test_nameko_sentry.py @@ -1,6 +1,7 @@ import logging import pytest +from eventlet.queue import Queue from mock import Mock, call, patch from nameko.containers import WorkerContext from nameko.extensions import Entrypoint @@ -83,6 +84,9 @@ def test_setup(reporter): transport = reporter.client.remote.get_transport() assert isinstance(transport, ThreadedHTTPTransport) + # queue created + assert isinstance(reporter.queue, Queue) + def test_setup_without_optional_config(config): @@ -100,6 +104,9 @@ def test_setup_without_optional_config(config): transport = reporter.client.remote.get_transport() assert isinstance(transport, ThreadedHTTPTransport) + # queue created + assert isinstance(reporter.queue, Queue) + def test_disabled(config): config['SENTRY']['DSN'] = None From fb2eb923b8ef7b57301223ff0e9b9d12b96ae341 Mon Sep 17 00:00:00 2001 From: Matt Bennett Date: Wed, 30 Dec 2015 15:27:08 +0000 Subject: [PATCH 7/9] update comment since stop isn't strictly a no-op --- test_nameko_sentry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_nameko_sentry.py b/test_nameko_sentry.py index 33e8106..2f1bdaa 100644 --- a/test_nameko_sentry.py +++ b/test_nameko_sentry.py @@ -217,7 +217,7 @@ def test_stop(container_factory, service_cls, config): reporter.stop() assert reporter._gt.dead - # subsequent stop should be a no-op + # subsequent stop has no adverse effect reporter.stop() From bfcafd371882110d57cf00a2002598c2614208ff Mon Sep 17 00:00:00 2001 From: Matt Bennett Date: Wed, 30 Dec 2015 15:28:55 +0000 Subject: [PATCH 8/9] use simpler mocking and assertions in test_start (no need to re-test the behavour of _run) --- test_nameko_sentry.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test_nameko_sentry.py b/test_nameko_sentry.py index 2f1bdaa..618043b 100644 --- a/test_nameko_sentry.py +++ b/test_nameko_sentry.py @@ -1,12 +1,12 @@ import logging import pytest +from eventlet.event import Event from eventlet.queue import Queue from mock import Mock, call, patch from nameko.containers import WorkerContext from nameko.extensions import Entrypoint from nameko.testing.services import dummy, entrypoint_hook, entrypoint_waiter -from nameko.testing.utils import wait_for_call # TODO: new lib from nameko.testing.utils import get_extension from nameko_sentry import SentryReporter from raven.transport.threaded import ThreadedHTTPTransport @@ -197,13 +197,17 @@ def test_start(container_factory, service_cls, config): reporter.setup() - with patch.object(reporter, 'queue') as queue: + running = Event() + + def run(): + running.send(True) + + with patch.object(reporter, '_run', wraps=run) as patched_run: reporter.start() - with wait_for_call(1, queue.get): - pass + running.wait() + assert patched_run.call_count == 1 assert reporter._gt is not None - assert queue.get.called def test_stop(container_factory, service_cls, config): From c9b89eb0a826f1dbca7f5a623c421b171c6a1d85 Mon Sep 17 00:00:00 2001 From: Matt Bennett Date: Wed, 30 Dec 2015 15:29:07 +0000 Subject: [PATCH 9/9] bump version --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 64fe2e4..f4caed5 100644 --- a/setup.py +++ b/setup.py @@ -3,7 +3,7 @@ setup( name='nameko-sentry', - version='0.0.1', + version='0.0.2', description='Nameko extension sends entrypoint exceptions to sentry', author='Matt Bennett', author_email='matt@bennett.name',