From 8b31b2e9b0c45aafcc55f3be2b59416e46c72d01 Mon Sep 17 00:00:00 2001 From: kgriffs Date: Tue, 7 May 2013 17:43:34 -0400 Subject: [PATCH] refactor: Hoist helpers.expect into package namespace It's silly to have to import a package's submodule directly, since in this case that only introduces another module name that the developer has to keep track of, just to gain access to a solitary function. This patch hoists marconi.tests.util.helpers.expect into marocni.tests.util so that it can be referenced more naturally by the test author. Accordingly, the Hacking.rst guide has been updated to allow this sort of thing (with some discretion). Implements: blueprint grizzly-debt Change-Id: Iad2e4f728aa4826d99bec2c599e2947a87eb5945 --- HACKING.rst | 7 ++++--- marconi/tests/storage/base.py | 19 +++++++++---------- marconi/tests/test_config.py | 3 +-- marconi/tests/util/__init__.py | 3 ++- marconi/tests/util/helpers.py | 19 ++++++++++++++++++- 5 files changed, 34 insertions(+), 17 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 86a95d97b..5b6e235d7 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -56,12 +56,13 @@ Example:: Imports ------- +- Only modules may be imported - Do not make relative imports - Order your imports by the full module path -- Import of classes is just allowed inside __init__ files. -- Organize your imports according to the following template +- Classes and functions may be hoisted into a package namespace, via __init__ files, with some discretion. +- Organize your imports according to the template given below -Example:: +Template:: {{stdlib imports in human alphabetical order}} \n diff --git a/marconi/tests/storage/base.py b/marconi/tests/storage/base.py index 59665469f..54c3d3ada 100644 --- a/marconi/tests/storage/base.py +++ b/marconi/tests/storage/base.py @@ -17,7 +17,6 @@ from marconi import storage from marconi.storage import exceptions from marconi.tests import util as testing -from marconi.tests.util import helpers class ControllerBaseTest(testing.TestBase): @@ -201,7 +200,7 @@ class MessageControllerTest(ControllerBaseTest): [msg1, msg2] = msgs # A wrong claim does not ensure the message deletion - with helpers.expected(storage.exceptions.NotPermitted): + with testing.expect(storage.exceptions.NotPermitted): self.controller.delete(self.queue_name, msg1["id"], project=self.project, claim=another_cid) @@ -211,7 +210,7 @@ class MessageControllerTest(ControllerBaseTest): project=self.project, claim=cid) - with helpers.expected(storage.exceptions.DoesNotExist): + with testing.expect(storage.exceptions.DoesNotExist): self.controller.get(self.queue_name, msg1["id"], project=self.project) @@ -224,7 +223,7 @@ class MessageControllerTest(ControllerBaseTest): self.claim_controller.delete(self.queue_name, cid, project=self.project) - with helpers.expected(storage.exceptions.NotPermitted): + with testing.expect(storage.exceptions.NotPermitted): self.controller.delete(self.queue_name, msg2["id"], project=self.project, claim=cid) @@ -236,7 +235,7 @@ class MessageControllerTest(ControllerBaseTest): project=self.project, client_uuid='my_uuid') - with helpers.expected(storage.exceptions.DoesNotExist): + with testing.expect(storage.exceptions.DoesNotExist): self.controller.get(self.queue_name, msgid, project=self.project) @@ -255,7 +254,7 @@ class MessageControllerTest(ControllerBaseTest): self.assertEquals(len(msgs), 0) - with helpers.expected(exceptions.DoesNotExist): + with testing.expect(exceptions.DoesNotExist): self.controller.get('unused', 'illformed', '480924') def test_illformed_claim(self): @@ -265,7 +264,7 @@ class MessageControllerTest(ControllerBaseTest): project='480924', client_uuid='unused') - with helpers.expected(exceptions.NotPermitted): + with testing.expect(exceptions.NotPermitted): self.controller.delete('unused', msgid, project='480924', claim='illformed') @@ -354,11 +353,11 @@ class ClaimControllerTest(ControllerBaseTest): claim_id, messages = self.controller.create(self.queue_name, meta, project=self.project) - with helpers.expected(storage.exceptions.DoesNotExist): + with testing.expect(storage.exceptions.DoesNotExist): self.controller.get(self.queue_name, claim_id, project=self.project) - with helpers.expected(storage.exceptions.DoesNotExist): + with testing.expect(storage.exceptions.DoesNotExist): self.controller.update(self.queue_name, claim_id, meta, project=self.project) @@ -368,7 +367,7 @@ class ClaimControllerTest(ControllerBaseTest): self.queue_controller.upsert('unused', {}, '480924') self.controller.delete('unused', 'illformed', '480924') - with helpers.expected(exceptions.DoesNotExist): + with testing.expect(exceptions.DoesNotExist): self.controller.update('unused', 'illformed', {'ttl': 40}, '480924') diff --git a/marconi/tests/test_config.py b/marconi/tests/test_config.py index d1cfa8ac7..cf46c4933 100644 --- a/marconi/tests/test_config.py +++ b/marconi/tests/test_config.py @@ -15,7 +15,6 @@ from marconi.common import config from marconi.tests import util as testing -from marconi.tests.util import helpers cfg_handle = config.project() @@ -36,5 +35,5 @@ class TestConfig(testing.TestBase): def test_wrong_type(self): ns = config.namespace('local') - with helpers.expected(config.cfg.Error): + with testing.expect(config.cfg.Error): ns.from_options(opt={}) diff --git a/marconi/tests/util/__init__.py b/marconi/tests/util/__init__.py index f8bc08d06..6a566a6cc 100644 --- a/marconi/tests/util/__init__.py +++ b/marconi/tests/util/__init__.py @@ -1,6 +1,7 @@ """Test utilities""" from marconi.tests.util import base - +from marconi.tests.util import helpers TestBase = base.TestBase +expect = helpers.expect diff --git a/marconi/tests/util/helpers.py b/marconi/tests/util/helpers.py index f42f51ea3..270d2ef0f 100644 --- a/marconi/tests/util/helpers.py +++ b/marconi/tests/util/helpers.py @@ -17,7 +17,24 @@ import contextlib @contextlib.contextmanager -def expected(*exc_type): +def expect(*exc_type): + """A context manager to validate raised expections. + + Can be used as an alternative to testtools.ExpectedException. + + Notable differences: + 1. This context manager accepts child classes of the + given type, testing that an "except" statement + referencing the given type would indeed catch it when + raised by the statement(s) defined inside the context. + 2. When the expected exception (or a child thereof) is + not raised, this context manager *always* raises + an AssertionError, both when a different exception + is raised, and when no exception is raised at all. + + :param *exc_type: Exception type(s) expected to be raised during + execution of the "with" context. + """ assert len(exc_type) > 0 try: