OpenStack Compute (nova)

Merge lp:~eday/nova/api-tests into lp:~hudson-openstack/nova/trunk

  1. api-tests
  2. Merge into trunk
Proposed by Eric Day
Status: Merged
Approved by: Eric Day
Approved revision: 253
Merged at revision: 249
Proposed branch: lp:~eday/nova/api-tests
Merge into: lp:~hudson-openstack/nova/trunk
Prerequisite: lp:~eday/nova/api-port
Diff against target: 304 lines (+192/-7) (has conflicts)
5 files modified
nova/api/__init__.py (+6/-0)
nova/api/test.py (+61/-0)
nova/wsgi.py (+12/-5)
nova/wsgi_test.py (+96/-0)
pylintrc (+17/-2)
Text conflict in nova/api/__init__.py
Text conflict in nova/wsgi.py
Text conflict in pylintrc
To merge this branch: bzr merge lp:~eday/nova/api-tests
Reviewer Review Type Date Requested Status
Matt Dietz (community) Approve
Review via email: mp+32960@code.launchpad.net

Commit message

Added unit tests for WSGI helpers and base WSGI API.

Description of the change

Added unit tests for WSGI helpers and base WSGI API.

To post a comment you must log in.
Revision history for this message
Michael Gundlach (gundlach) wrote :

257: Maybe accept id explicitly to also verify that nothing was passed except for the id?

282: Replace all this with webob.Request.blank('/').environ . You can also use Request(environ).get_response(some_wsgi_app) to simplify some of your code if you wish. Since these tests can assume that webob works properly (they're not testing webob), I don't see a reason not to use it to simplify the WSGI.

I am not totally comfortable with the self.called approach to testing, but I can't think of anything better, so that's fine :)

I see you changed 'disable' back to 'disable-msg' which is fine; I need to change to 'disable' across the whole project, in a separate branch, now that pylint 0.21.1 has changed their API.

Revision history for this message
Eric Day (eday) wrote :

Ahh, great points on using the webob stuff, I'm play around with those and resubmit. I agree the self.called isn't the best looking solution, but because having to match the environ directly, mox wasn't trivial to use (more code/setup). I'll try to think of something cleaner.

Revision history for this message
Eric Day (eday) wrote :

Ready for another review, used webob to make things quite a bit simpler.

lp:~eday/nova/api-tests updated
252. By Eric Day

Added '-' as possible charater in module rgx.

253. By Eric Day

More pylintrc updates.

Revision history for this message
Michael Gundlach (gundlach) wrote :

nova/api/__init__.py and some others have got merge conflict code still in them e.g. >>>>>>>>>>> TREE.

Revision history for this message
Michael Gundlach (gundlach) wrote :

is C0103 actually firing on def show(self, req, id)? If so, great, but I thought maybe you left it there by accident.

Revision history for this message
Eric Day (eday) wrote :

Yeah, the warning is firing because 'id' is a reserved keyword, but routes generates this automatically. Only thing to do is suppress. The merge conflicts are not always accurate with what will happen in trunk, LP is doing something else on the back-end.

Revision history for this message
Matt Dietz (cerberus) wrote :

As per discussion in #openstack, this looks like a great start for testing the API.

review: Approve
Revision history for this message
Michael Gundlach (gundlach) wrote :

So even though your diff showed a bunch of added lines like
">>>>>>>>>>>>>>>>>> TREE", those aren't actually part of the diff that will
be applied? I don't know how to review the code if that's the case...

On Thu, Aug 19, 2010 at 12:24 PM, Eric Day <email address hidden> wrote:

> Yeah, the warning is firing because 'id' is a reserved keyword, but routes
> generates this automatically. Only thing to do is suppress. The merge
> conflicts are not always accurate with what will happen in trunk, LP is
> doing something else on the back-end.
> --
> https://code.launchpad.net/~eday/nova/api-tests/+merge/32960
> Your team Nova Core is requested to review the proposed merge of
> lp:~eday/nova/api-tests into lp:nova.
>

Confidentiality Notice: This e-mail message (including any attached or
embedded documents) is intended for the exclusive and confidential use of the
individual or entity to which this message is addressed, and unless otherwise
expressly indicated, is confidential and privileged information of Rackspace.
Any dissemination, distribution or copying of the enclosed material is prohibited.
If you receive this transmission in error, please notify us immediately by e-mail
at <email address hidden>, and delete the original message.
Your cooperation is appreciated.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/api/__init__.py'
2--- nova/api/__init__.py 2010-08-17 23:43:37 +0000
3+++ nova/api/__init__.py 2010-08-19 01:41:08 +0000
4@@ -32,7 +32,13 @@
5
6 def __init__(self):
7 mapper = routes.Mapper()
8+<<<<<<< TREE
9 mapper.connect(None, "/v1.0/{path_info:.*}",
10 controller=rackspace.API())
11 mapper.connect(None, "/ec2/{path_info:.*}", controller=ec2.API())
12 super(API, self).__init__(mapper)
13+=======
14+ mapper.connect("/v1.0/{path_info:.*}", controller=rackspace.API())
15+ mapper.connect("/ec2/{path_info:.*}", controller=ec2.API())
16+ super(API, self).__init__(mapper)
17+>>>>>>> MERGE-SOURCE
18
19=== added file 'nova/api/test.py'
20--- nova/api/test.py 1970-01-01 00:00:00 +0000
21+++ nova/api/test.py 2010-08-19 01:41:08 +0000
22@@ -0,0 +1,61 @@
23+# vim: tabstop=4 shiftwidth=4 softtabstop=4
24+
25+# Copyright 2010 OpenStack LLC.
26+# All Rights Reserved.
27+#
28+# Licensed under the Apache License, Version 2.0 (the "License"); you may
29+# not use this file except in compliance with the License. You may obtain
30+# a copy of the License at
31+#
32+# http://www.apache.org/licenses/LICENSE-2.0
33+#
34+# Unless required by applicable law or agreed to in writing, software
35+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
36+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
37+# License for the specific language governing permissions and limitations
38+# under the License.
39+
40+"""
41+Test for the root WSGI middleware for all API controllers.
42+"""
43+
44+import unittest
45+
46+import stubout
47+import webob
48+import webob.dec
49+
50+from nova import api
51+
52+
53+class Test(unittest.TestCase):
54+
55+ def setUp(self): # pylint: disable-msg=C0103
56+ self.stubs = stubout.StubOutForTesting()
57+
58+ def tearDown(self): # pylint: disable-msg=C0103
59+ self.stubs.UnsetAll()
60+
61+ def test_rackspace(self):
62+ self.stubs.Set(api.rackspace, 'API', APIStub)
63+ result = webob.Request.blank('/v1.0/cloud').get_response(api.API())
64+ self.assertEqual(result.body, "/cloud")
65+
66+ def test_ec2(self):
67+ self.stubs.Set(api.ec2, 'API', APIStub)
68+ result = webob.Request.blank('/ec2/cloud').get_response(api.API())
69+ self.assertEqual(result.body, "/cloud")
70+
71+ def test_not_found(self):
72+ self.stubs.Set(api.ec2, 'API', APIStub)
73+ self.stubs.Set(api.rackspace, 'API', APIStub)
74+ result = webob.Request.blank('/test/cloud').get_response(api.API())
75+ self.assertNotEqual(result.body, "/cloud")
76+
77+
78+class APIStub(object):
79+ """Class to verify request and mark it was called."""
80+
81+ @webob.dec.wsgify
82+ def __call__(self, req):
83+ return req.path_info
84
85=== modified file 'nova/wsgi.py'
86--- nova/wsgi.py 2010-08-18 15:44:24 +0000
87+++ nova/wsgi.py 2010-08-19 01:41:08 +0000
88@@ -83,7 +83,11 @@
89 raise NotImplementedError("You must implement __call__")
90
91
92+<<<<<<< TREE
93 class Middleware(Application): # pylint: disable-msg=W0223
94+=======
95+class Middleware(Application):
96+>>>>>>> MERGE-SOURCE
97 """
98 Base WSGI middleware wrapper. These classes require an application to be
99 initialized that will be called next. By default the middleware will
100@@ -95,7 +99,7 @@
101 self.application = application
102
103 @webob.dec.wsgify
104- def __call__(self, req):
105+ def __call__(self, req): # pylint: disable-msg=W0221
106 """Override to implement middleware behavior."""
107 return self.application
108
109@@ -113,7 +117,7 @@
110 resp = req.get_response(self.application)
111
112 print ("*" * 40) + " RESPONSE HEADERS"
113- for (key, value) in resp.headers:
114+ for (key, value) in resp.headers.iteritems():
115 print key, "=", value
116 print
117
118@@ -127,7 +131,7 @@
119 Iterator that prints the contents of a wrapper string iterator
120 when iterated.
121 """
122- print ("*" * 40) + "BODY"
123+ print ("*" * 40) + " BODY"
124 for part in app_iter:
125 sys.stdout.write(part)
126 sys.stdout.flush()
127@@ -176,8 +180,9 @@
128 """
129 return self._router
130
131+ @staticmethod
132 @webob.dec.wsgify
133- def _dispatch(self, req):
134+ def _dispatch(req):
135 """
136 Called by self._router after matching the incoming request to a route
137 and putting the information into req.environ. Either returns 404
138@@ -197,6 +202,7 @@
139 must, in addition to their normal parameters, accept a 'req' argument
140 which is the incoming webob.Request.
141 """
142+
143 @webob.dec.wsgify
144 def __call__(self, req):
145 """
146@@ -249,6 +255,7 @@
147 return repr(data)
148
149 def _to_xml_node(self, doc, metadata, nodename, data):
150+ """Recursive method to convert data members to XML nodes."""
151 result = doc.createElement(nodename)
152 if type(data) is list:
153 singular = metadata.get('plurals', {}).get(nodename, None)
154@@ -262,7 +269,7 @@
155 result.appendChild(node)
156 elif type(data) is dict:
157 attrs = metadata.get('attributes', {}).get(nodename, {})
158- for k,v in data.items():
159+ for k, v in data.items():
160 if k in attrs:
161 result.setAttribute(k, str(v))
162 else:
163
164=== added file 'nova/wsgi_test.py'
165--- nova/wsgi_test.py 1970-01-01 00:00:00 +0000
166+++ nova/wsgi_test.py 2010-08-19 01:41:08 +0000
167@@ -0,0 +1,96 @@
168+# vim: tabstop=4 shiftwidth=4 softtabstop=4
169+
170+# Copyright 2010 United States Government as represented by the
171+# Administrator of the National Aeronautics and Space Administration.
172+# Copyright 2010 OpenStack LLC.
173+# All Rights Reserved.
174+#
175+# Licensed under the Apache License, Version 2.0 (the "License"); you may
176+# not use this file except in compliance with the License. You may obtain
177+# a copy of the License at
178+#
179+# http://www.apache.org/licenses/LICENSE-2.0
180+#
181+# Unless required by applicable law or agreed to in writing, software
182+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
183+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
184+# License for the specific language governing permissions and limitations
185+# under the License.
186+
187+"""
188+Test WSGI basics and provide some helper functions for other WSGI tests.
189+"""
190+
191+import unittest
192+
193+import routes
194+import webob
195+
196+from nova import wsgi
197+
198+
199+class Test(unittest.TestCase):
200+
201+ def test_debug(self):
202+
203+ class Application(wsgi.Application):
204+ """Dummy application to test debug."""
205+
206+ def __call__(self, environ, start_response):
207+ start_response("200", [("X-Test", "checking")])
208+ return ['Test result']
209+
210+ application = wsgi.Debug(Application())
211+ result = webob.Request.blank('/').get_response(application)
212+ self.assertEqual(result.body, "Test result")
213+
214+ def test_router(self):
215+
216+ class Application(wsgi.Application):
217+ """Test application to call from router."""
218+
219+ def __call__(self, environ, start_response):
220+ start_response("200", [])
221+ return ['Router result']
222+
223+ class Router(wsgi.Router):
224+ """Test router."""
225+
226+ def __init__(self):
227+ mapper = routes.Mapper()
228+ mapper.connect("/test", controller=Application())
229+ super(Router, self).__init__(mapper)
230+
231+ result = webob.Request.blank('/test').get_response(Router())
232+ self.assertEqual(result.body, "Router result")
233+ result = webob.Request.blank('/bad').get_response(Router())
234+ self.assertNotEqual(result.body, "Router result")
235+
236+ def test_controller(self):
237+
238+ class Controller(wsgi.Controller):
239+ """Test controller to call from router."""
240+ test = self
241+
242+ def show(self, req, id): # pylint: disable-msg=W0622,C0103
243+ """Default action called for requests with an ID."""
244+ self.test.assertEqual(req.path_info, '/tests/123')
245+ self.test.assertEqual(id, '123')
246+ return id
247+
248+ class Router(wsgi.Router):
249+ """Test router."""
250+
251+ def __init__(self):
252+ mapper = routes.Mapper()
253+ mapper.resource("test", "tests", controller=Controller())
254+ super(Router, self).__init__(mapper)
255+
256+ result = webob.Request.blank('/tests/123').get_response(Router())
257+ self.assertEqual(result.body, "123")
258+ result = webob.Request.blank('/test/123').get_response(Router())
259+ self.assertNotEqual(result.body, "123")
260+
261+ def test_serializer(self):
262+ # TODO(eday): Placeholder for serializer testing.
263+ pass
264
265=== modified file 'pylintrc'
266--- pylintrc 2010-08-18 16:09:29 +0000
267+++ pylintrc 2010-08-19 01:41:08 +0000
268@@ -1,19 +1,34 @@
269 [Messages Control]
270+<<<<<<< TREE
271 disable-msg=C0103
272 # TODOs in code comments are fine...
273 disable-msg=W0511
274 # *args and **kwargs are fine
275 disable-msg=W0142
276+=======
277+# W0511: TODOs in code comments are fine.
278+# W0142: *args and **kwargs are fine.
279+disable-msg=W0511,W0142
280+>>>>>>> MERGE-SOURCE
281
282 [Basic]
283-# Variables can be 1 to 31 characters long, with
284-# lowercase and underscores
285+# Variable names can be 1 to 31 characters long, with lowercase and underscores
286 variable-rgx=[a-z_][a-z0-9_]{0,30}$
287
288+# Argument names can be 2 to 31 characters long, with lowercase and underscores
289+argument-rgx=[a-z_][a-z0-9_]{1,30}$
290+
291 # Method names should be at least 3 characters long
292 # and be lowecased with underscores
293 method-rgx=[a-z_][a-z0-9_]{2,50}$
294
295+# Module names matching nova-* are ok (files in bin/)
296+module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+)|(nova-[a-z0-9_-]+))$
297+
298+# Don't require docstrings on tests.
299+no-docstring-rgx=((__.*__)|([tT]est.*)|setUp|tearDown)$
300+
301 [Design]
302 max-public-methods=100
303 min-public-methods=0
304+max-args=6