Merge lp:~free.ekanayaka/python-zope-fixtures/adapter-fixture into lp:python-zope-fixtures

Proposed by Free Ekanayaka
Status: Merged
Merged at revision: 8
Proposed branch: lp:~free.ekanayaka/python-zope-fixtures/adapter-fixture
Merge into: lp:python-zope-fixtures
Diff against target: 239 lines (+151/-13)
3 files modified
lib/zope_fixtures/__init__.py (+3/-1)
lib/zope_fixtures/components.py (+77/-9)
lib/zope_fixtures/tests/test_components.py (+71/-3)
To merge this branch: bzr merge lp:~free.ekanayaka/python-zope-fixtures/adapter-fixture
Reviewer Review Type Date Requested Status
Robert Collins Approve
Review via email: mp+124187@code.launchpad.net

Description of the change

I realized that the ComponentsFixture might not be a good idea, or at least it probably doesn't match the typical use cases one would have in a Zope-based application.

The reason is that it uses the low-level sethook() API to override the current registry (aka site manager), while Zope actually supports an higher-level API in the zope.components.hooks module that seems precisely meant to support thread-local/request-specific overrides via the setHooks() and setSite() APIs. This higher-level API is indeed implemented on top of sethook().

I believe ComponentsFixture should be dropped and maybe replaced with a SiteFixture which would use the setHooks/setSite APIs to offer a similar behavior, but more in line with what the Zope publisher does, and hence more suited for writing unit-tests.

Said that, this branch heads into that direction and re-implements UtilityFixture without using ComponentsFixture under the hood, and merely using whatever site manager is currently in-force (thus letting test code be in control of the site manager). It also adds an AdapterFixture which pretty much does the same, but with adapters.

It should be possible to virtually get the same features of ComponentsFixture using UtilityFixture and AdapterFixture.

To post a comment you must log in.
Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

FWIW more information on the canonical use of hooks in a Zope application can be found in zope/component/hooks.txt and zope/site/site.txt.

Revision history for this message
Robert Collins (lifeless) wrote :

I think this:
76 +
77 + # Check if an utility for the same interface and name exists, if so
78 + # restore it upon cleanup, otherwise just unregister our one.
79 + original = sm.queryUtility(self._provided, self._name)
80 + if original is not None:
81 + self.addCleanup(sm.registerUtility, original, self._provided,
82 + self._name)
83 + else:
84 + self.addCleanup(sm.unregisterUtility, self._component,
85 + self._provided, self._name)
86 +
87 + # Register the utility
88 + sm.registerUtility(self._component, self._provided, self._name)
might be clearer as
76 +
79 + original = sm.queryUtility(self._provided, self._name)
88 + sm.registerUtility(self._component, self._provided, self._name)
80 + if original is not None:
77 + # Restore any existing utility.
81 + self.addCleanup(sm.registerUtility, original, self._provided,
82 + self._name)
87 + # Register the utility
84 + self.addCleanup(sm.unregisterUtility, self._component,
85 + self._provided, self._name)

Which due to the LIFO nature of cleanups will just always unregister the utility we are using, and restore any previous utility - this is easier to reason about - it doesn't depend on the silent-replace aspect of registerUtility, and the perf cost is trivial (one call to unregisterUtility).

48 + The utility will be unregistered upon cleanUp, or the original utility
49 + will be restored if there was one.

Perhaps: "The registration will be reverted upon cleanUp, restoring the previous utility (or None)."

same pattern in adapterFixture. I wonder if we can pull that logic sideways and make it independent.

I'll merge what you have and see about a refactoring branch.

review: Approve
Revision history for this message
Robert Collins (lifeless) wrote :

Ok, so it comes together okish, but not enough better to worry about just yet - I think if callmany were more easily accessible it would be that little bit better and worth doing.

Revision history for this message
Robert Collins (lifeless) wrote :

Also, you forgot a NEWS entry ;).

Revision history for this message
Robert Collins (lifeless) wrote :

BTW, do you have more changes, or should we release this?

Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

Hi Robert,

thanks for the careful review. Yeah, I thought too about unifying the logic but it just looked not enough worth yet as you point. I'm fine with the proposed change in the addCleanup calls, if you feel like committing it go ahead. Good catch on the NEWS entry, and yes please release it, I don't have further changes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/zope_fixtures/__init__.py'
--- lib/zope_fixtures/__init__.py 2012-04-26 02:58:05 +0000
+++ lib/zope_fixtures/__init__.py 2012-09-13 12:45:41 +0000
@@ -36,9 +36,11 @@
36__all__ = [36__all__ = [
37 'ComponentsFixture',37 'ComponentsFixture',
38 'UtilityFixture',38 'UtilityFixture',
39 'AdapterFixture',
39 ]40 ]
4041
41from zope_fixtures.components import ComponentsFixture, UtilityFixture42from zope_fixtures.components import (
43 ComponentsFixture, UtilityFixture, AdapterFixture)
4244
4345
44def test_suite():46def test_suite():
4547
=== modified file 'lib/zope_fixtures/components.py'
--- lib/zope_fixtures/components.py 2012-04-26 02:52:29 +0000
+++ lib/zope_fixtures/components.py 2012-09-13 12:45:41 +0000
@@ -16,6 +16,7 @@
16__all__ = [16__all__ = [
17 'ComponentsFixture',17 'ComponentsFixture',
18 'UtilityFixture',18 'UtilityFixture',
19 'AdapterFixture',
19 ]20 ]
2021
21from fixtures import Fixture22from fixtures import Fixture
@@ -23,6 +24,9 @@
2324
24Components = try_import("zope.component.registry.Components")25Components = try_import("zope.component.registry.Components")
25getSiteManager = try_import("zope.component.getSiteManager")26getSiteManager = try_import("zope.component.getSiteManager")
27_getUtilityProvided = try_import("zope.component.registry._getUtilityProvided")
28_getAdapterProvided = try_import("zope.component.registry._getAdapterProvided")
29_getAdapterRequired = try_import("zope.component.registry._getAdapterRequired")
2630
2731
28class ComponentsFixture(Fixture):32class ComponentsFixture(Fixture):
@@ -65,19 +69,83 @@
6569
6670
67class UtilityFixture(Fixture):71class UtilityFixture(Fixture):
68 """Convenience around ComponentsFixture to override a single utility."""72 """Fixture for registering or overriding a single utility.
6973
70 def __init__(self, *args, **kwargs):74 The utility will be unregistered upon cleanUp, or the original utility
75 will be restored if there was one.
76
77 The registration is performed agaist the current site manager.
78 """
79
80 def __init__(self, component, provided=None, name=u""):
71 """Create an UtilityFixture.81 """Create an UtilityFixture.
7282
73 The args and kwargs parameters are the same supported by83 The parameters are the same supported by registerUtility and will
74 registerUtility and will be passed to it.84 be passed to it.
75 """85 """
76 super(UtilityFixture, self).__init__()86 super(UtilityFixture, self).__init__()
77 self.args = args87 self._component = component
78 self.kwargs = kwargs88 self._provided = provided or _getUtilityProvided(component)
89 self._name = name
7990
80 def setUp(self):91 def setUp(self):
81 super(UtilityFixture, self).setUp()92 super(UtilityFixture, self).setUp()
82 components = self.useFixture(ComponentsFixture())93 # We use the current site manager so we honor whatever hooks have
83 components.registry.registerUtility(*self.args, **self.kwargs)94 # been set
95 sm = getSiteManager()
96
97 # Check if an utility for the same interface and name exists, if so
98 # restore it upon cleanup, otherwise just unregister our one.
99 original = sm.queryUtility(self._provided, self._name)
100 if original is not None:
101 self.addCleanup(sm.registerUtility, original, self._provided,
102 self._name)
103 else:
104 self.addCleanup(sm.unregisterUtility, self._component,
105 self._provided, self._name)
106
107 # Register the utility
108 sm.registerUtility(self._component, self._provided, self._name)
109
110
111class AdapterFixture(Fixture):
112 """Fixture for registering or overriding a single adapter.
113
114 The adapter will be unregistered upon cleanUp, or the original adapter
115 will be restored if there was one.
116
117 The registration is performed agaist the current site manager.
118 """
119
120 def __init__(self, factory, required=None, provided=None, name=u""):
121 """Create an AdapterFixture.
122
123 The parameters are the same supported by registerAdpater and will
124 be passed to it.
125 """
126 super(AdapterFixture, self).__init__()
127 self._factory = factory
128 self._required = _getAdapterRequired(factory, required)
129 self._provided = provided or _getAdapterProvided(factory)
130 self._name = name
131
132 def setUp(self):
133 super(AdapterFixture, self).setUp()
134 # We use the current site manager so we honor whatever hooks have
135 # been set
136 sm = getSiteManager()
137
138 # Check if an utility for the same interface and name exists, if so
139 # restore it upon cleanup, otherwise just unregister our one.
140 original = sm.adapters.registered(self._required, self._provided,
141 self._name)
142 if original is not None:
143 self.addCleanup(sm.registerAdapter, original, self._required,
144 self._provided, self._name)
145 else:
146 self.addCleanup(sm.unregisterAdapter, self._factory, self._required,
147 self._provided, self._name)
148
149 # Register the utility
150 sm.registerAdapter(self._factory, self._required, self._provided,
151 self._name)
84152
=== modified file 'lib/zope_fixtures/tests/test_components.py'
--- lib/zope_fixtures/tests/test_components.py 2012-04-26 02:52:29 +0000
+++ lib/zope_fixtures/tests/test_components.py 2012-09-13 12:45:41 +0000
@@ -16,10 +16,12 @@
16import testtools16import testtools
1717
18from zope.interface import Interface, implements18from zope.interface import Interface, implements
19from zope.component import provideUtility, queryUtility, getSiteManager19from zope.component import (
20 provideUtility, queryUtility, getSiteManager, queryAdapter, adapts)
20from zope.component.registry import Components21from zope.component.registry import Components
2122
22from zope_fixtures import ComponentsFixture, UtilityFixture23from zope_fixtures import (
24 ComponentsFixture, UtilityFixture, AdapterFixture)
2325
2426
25class ITestUtility(Interface):27class ITestUtility(Interface):
@@ -30,6 +32,26 @@
30 implements(ITestUtility)32 implements(ITestUtility)
3133
3234
35class ITestContext(Interface):
36 pass
37
38
39class TestContext(object):
40 implements(ITestContext)
41
42
43class ITestAdapter(Interface):
44 pass
45
46
47class TestAdapter(object):
48 implements(ITestAdapter)
49 adapts(ITestContext)
50
51 def __init__(self, context):
52 self.context = context
53
54
33class TestComponentsFixture(testtools.TestCase):55class TestComponentsFixture(testtools.TestCase):
3456
35 def test_overlays_site_manager(self):57 def test_overlays_site_manager(self):
@@ -98,14 +120,60 @@
98 self.useFixture(fixture)120 self.useFixture(fixture)
99 self.assertIs(utility, queryUtility(ITestUtility))121 self.assertIs(utility, queryUtility(ITestUtility))
100122
101 def test_restore_utility(self):123 def test_unregister_utility(self):
102 utility = TestUtility()124 utility = TestUtility()
103 with UtilityFixture(utility):125 with UtilityFixture(utility):
104 self.assertIs(utility, queryUtility(ITestUtility))126 self.assertIs(utility, queryUtility(ITestUtility))
105 self.assertIs(None, queryUtility(ITestUtility))127 self.assertIs(None, queryUtility(ITestUtility))
106128
129 def test_restore_original_utility(self):
130 original = TestUtility()
131 sm = getSiteManager()
132 sm.registerUtility(original)
133 self.addCleanup(sm.unregisterUtility, original)
134 utility = TestUtility()
135 with UtilityFixture(utility):
136 self.assertIs(utility, queryUtility(ITestUtility))
137 self.assertIs(original, queryUtility(ITestUtility))
138
107 def test_arguments_passthrough(self):139 def test_arguments_passthrough(self):
108 utility = TestUtility()140 utility = TestUtility()
109 fixture = UtilityFixture(utility, name="foo")141 fixture = UtilityFixture(utility, name="foo")
110 self.useFixture(fixture)142 self.useFixture(fixture)
111 self.assertIs(utility, queryUtility(ITestUtility, name="foo"))143 self.assertIs(utility, queryUtility(ITestUtility, name="foo"))
144
145
146class TestAdapterFixture(testtools.TestCase):
147
148 def test_provide_adapter(self):
149 fixture = AdapterFixture(TestAdapter)
150 self.useFixture(fixture)
151 context = TestContext()
152 adapter = queryAdapter(context)
153 self.assertIs(context, adapter.context)
154
155 def test_unregister_adapter(self):
156 context = TestContext()
157 with AdapterFixture(TestAdapter):
158 adapter = queryAdapter(context)
159 self.assertIs(context, adapter.context)
160 self.assertIs(None, queryAdapter(context))
161
162 def test_restore_original_adapter(self):
163
164 class TestAdapter2(TestAdapter):
165 pass
166
167 sm = getSiteManager()
168 sm.registerAdapter(TestAdapter2)
169 self.addCleanup(sm.unregisterAdapter, TestAdapter2)
170 context = TestContext()
171 with AdapterFixture(TestAdapter):
172 self.assertIsInstance(queryAdapter(context), TestAdapter)
173 self.assertIsInstance(queryAdapter(context), TestAdapter2)
174
175 def test_arguments_passthrough(self):
176 fixture = AdapterFixture(TestAdapter, name="foo")
177 self.useFixture(fixture)
178 context = TestContext()
179 self.assertIsInstance(queryAdapter(context, name="foo"), TestAdapter)

Subscribers

People subscribed via source and target branches