Launchpad itself

Code review comment for lp:~mwhudson/launchpad/vostok-add-root

  1. vostok-add-root
  2. Merge into devel
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

On Thu, 29 Jul 2010 23:27:43 -0000, Tim Penhey <email address hidden> wrote:
> Review: Needs Information
> > class VostokRootView(LaunchpadView):
> > pass
>
> Shouldn't you at least have
>
> __used_for__ = IVostokRoot

After a bit of googling, apparently not. Maybe I should sed out all the
existing __used_for__s in our code base :-)

> Instead of:
> view = getMultiAdapter(
> (VostokRoot(), VostokTestRequest()), name='+index')
> view.initialize()
>
> Can you alter the get_initialized_view function?

Apparently yes, once I was told that it's actually called
create_initialized_view :-)

> > class IVostokRoot(Interface): # might need to inherit from some IRoot thing
>
> You can probably lose the comment.

Yeah, thanks.

> Also... the root.pt needs some work :-)

That's in the next pipe :-)

> Why is the
> getUtility(IOpenLaunchBag).clear()
> needed in test_root_object?

Errr... because the test blows up if its not there. Specifically, the
root object seems to be cached on the launchbag, and if the launchpad
isn't set up, it blows up.

I don't know if this is still used at all, quite possibly not, but that
seems a job for another branch.

I've expanded the comment.

Cheers,
mwh

vostok-add-root-progress.diff
1=== modified file 'lib/lp/vostok/browser/root.py'
2--- lib/lp/vostok/browser/root.py 2010-07-15 07:49:25 +0000
3+++ lib/lp/vostok/browser/root.py 2010-07-29 23:45:26 +0000
4@@ -12,4 +12,4 @@
5
6
7 class VostokRootView(LaunchpadView):
8- pass
9+ """The view for the Vostok root object."""
10
11=== modified file 'lib/lp/vostok/browser/tests/test_root.py'
12--- lib/lp/vostok/browser/tests/test_root.py 2010-07-29 04:30:04 +0000
13+++ lib/lp/vostok/browser/tests/test_root.py 2010-07-29 23:51:17 +0000
14@@ -8,14 +8,15 @@
15 import unittest
16
17 from zope.app.publisher.browser import getDefaultViewName
18-from zope.component import getMultiAdapter
19
20 from canonical.testing.layers import FunctionalLayer
21
22 from lp.testing import TestCase
23+from lp.testing.views import create_initialized_view
24 from lp.vostok.browser.root import VostokRootView
25 from lp.vostok.browser.tests.request import VostokTestRequest
26-from lp.vostok.publisher import VostokRoot
27+from lp.vostok.publisher import VostokLayer, VostokRoot
28+
29
30 class TestBrowseRoot(TestCase):
31
32@@ -28,9 +29,8 @@
33
34 def test_root_index_view(self):
35 # VostokRootView is registered as the view for the VostokRoot object.
36- view = getMultiAdapter(
37- (VostokRoot(), VostokTestRequest()), name='+index')
38- view.initialize()
39+ view = create_initialized_view(
40+ VostokRoot(), name='+index', layer=VostokLayer)
41 self.assertIsInstance(view, VostokRootView)
42
43
44
45=== modified file 'lib/lp/vostok/publisher.py'
46--- lib/lp/vostok/publisher.py 2010-07-14 14:27:27 +0000
47+++ lib/lp/vostok/publisher.py 2010-07-29 23:39:07 +0000
48@@ -28,7 +28,7 @@
49 implements(VostokLayer)
50
51
52-class IVostokRoot(Interface): # might need to inherit from some IRoot thing
53+class IVostokRoot(Interface):
54 """Marker interface for the root vostok object."""
55
56
57
58=== modified file 'lib/lp/vostok/tests/test_publisher.py'
59--- lib/lp/vostok/tests/test_publisher.py 2010-07-29 04:28:08 +0000
60+++ lib/lp/vostok/tests/test_publisher.py 2010-07-30 00:01:34 +0000
61@@ -37,7 +37,8 @@
62 request, publication = get_request_and_publication(
63 host=config.vhost.vostok.hostname)
64 self.assertProvides(request, VostokLayer)
65- # XXX This shouldn't be needed:
66+ # XXX getApplication caches the root object in the LaunchBag, so we
67+ # need to set it up, or it crashes.
68 getUtility(IOpenLaunchBag).clear()
69 root = publication.getApplication(request)
70 self.assertIsInstance(root, VostokRoot)

« Back to merge proposal