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
=== modified file 'nova/api/__init__.py'
--- nova/api/__init__.py 2010-08-17 23:43:37 +0000
+++ nova/api/__init__.py 2010-08-19 01:41:08 +0000
@@ -32,7 +32,13 @@
3232
33 def __init__(self):33 def __init__(self):
34 mapper = routes.Mapper()34 mapper = routes.Mapper()
35<<<<<<< TREE
35 mapper.connect(None, "/v1.0/{path_info:.*}",36 mapper.connect(None, "/v1.0/{path_info:.*}",
36 controller=rackspace.API())37 controller=rackspace.API())
37 mapper.connect(None, "/ec2/{path_info:.*}", controller=ec2.API())38 mapper.connect(None, "/ec2/{path_info:.*}", controller=ec2.API())
38 super(API, self).__init__(mapper)39 super(API, self).__init__(mapper)
40=======
41 mapper.connect("/v1.0/{path_info:.*}", controller=rackspace.API())
42 mapper.connect("/ec2/{path_info:.*}", controller=ec2.API())
43 super(API, self).__init__(mapper)
44>>>>>>> MERGE-SOURCE
3945
=== added file 'nova/api/test.py'
--- nova/api/test.py 1970-01-01 00:00:00 +0000
+++ nova/api/test.py 2010-08-19 01:41:08 +0000
@@ -0,0 +1,61 @@
1# vim: tabstop=4 shiftwidth=4 softtabstop=4
2
3# Copyright 2010 OpenStack LLC.
4# All Rights Reserved.
5#
6# Licensed under the Apache License, Version 2.0 (the "License"); you may
7# not use this file except in compliance with the License. You may obtain
8# a copy of the License at
9#
10# http://www.apache.org/licenses/LICENSE-2.0
11#
12# Unless required by applicable law or agreed to in writing, software
13# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
14# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
15# License for the specific language governing permissions and limitations
16# under the License.
17
18"""
19Test for the root WSGI middleware for all API controllers.
20"""
21
22import unittest
23
24import stubout
25import webob
26import webob.dec
27
28from nova import api
29
30
31class Test(unittest.TestCase):
32
33 def setUp(self): # pylint: disable-msg=C0103
34 self.stubs = stubout.StubOutForTesting()
35
36 def tearDown(self): # pylint: disable-msg=C0103
37 self.stubs.UnsetAll()
38
39 def test_rackspace(self):
40 self.stubs.Set(api.rackspace, 'API', APIStub)
41 result = webob.Request.blank('/v1.0/cloud').get_response(api.API())
42 self.assertEqual(result.body, "/cloud")
43
44 def test_ec2(self):
45 self.stubs.Set(api.ec2, 'API', APIStub)
46 result = webob.Request.blank('/ec2/cloud').get_response(api.API())
47 self.assertEqual(result.body, "/cloud")
48
49 def test_not_found(self):
50 self.stubs.Set(api.ec2, 'API', APIStub)
51 self.stubs.Set(api.rackspace, 'API', APIStub)
52 result = webob.Request.blank('/test/cloud').get_response(api.API())
53 self.assertNotEqual(result.body, "/cloud")
54
55
56class APIStub(object):
57 """Class to verify request and mark it was called."""
58
59 @webob.dec.wsgify
60 def __call__(self, req):
61 return req.path_info
062
=== modified file 'nova/wsgi.py'
--- nova/wsgi.py 2010-08-18 15:44:24 +0000
+++ nova/wsgi.py 2010-08-19 01:41:08 +0000
@@ -83,7 +83,11 @@
83 raise NotImplementedError("You must implement __call__")83 raise NotImplementedError("You must implement __call__")
8484
8585
86<<<<<<< TREE
86class Middleware(Application): # pylint: disable-msg=W022387class Middleware(Application): # pylint: disable-msg=W0223
88=======
89class Middleware(Application):
90>>>>>>> MERGE-SOURCE
87 """91 """
88 Base WSGI middleware wrapper. These classes require an application to be92 Base WSGI middleware wrapper. These classes require an application to be
89 initialized that will be called next. By default the middleware will93 initialized that will be called next. By default the middleware will
@@ -95,7 +99,7 @@
95 self.application = application99 self.application = application
96100
97 @webob.dec.wsgify101 @webob.dec.wsgify
98 def __call__(self, req):102 def __call__(self, req): # pylint: disable-msg=W0221
99 """Override to implement middleware behavior."""103 """Override to implement middleware behavior."""
100 return self.application104 return self.application
101105
@@ -113,7 +117,7 @@
113 resp = req.get_response(self.application)117 resp = req.get_response(self.application)
114118
115 print ("*" * 40) + " RESPONSE HEADERS"119 print ("*" * 40) + " RESPONSE HEADERS"
116 for (key, value) in resp.headers:120 for (key, value) in resp.headers.iteritems():
117 print key, "=", value121 print key, "=", value
118 print122 print
119123
@@ -127,7 +131,7 @@
127 Iterator that prints the contents of a wrapper string iterator131 Iterator that prints the contents of a wrapper string iterator
128 when iterated.132 when iterated.
129 """133 """
130 print ("*" * 40) + "BODY"134 print ("*" * 40) + " BODY"
131 for part in app_iter:135 for part in app_iter:
132 sys.stdout.write(part)136 sys.stdout.write(part)
133 sys.stdout.flush()137 sys.stdout.flush()
@@ -176,8 +180,9 @@
176 """180 """
177 return self._router181 return self._router
178182
183 @staticmethod
179 @webob.dec.wsgify184 @webob.dec.wsgify
180 def _dispatch(self, req):185 def _dispatch(req):
181 """186 """
182 Called by self._router after matching the incoming request to a route187 Called by self._router after matching the incoming request to a route
183 and putting the information into req.environ. Either returns 404188 and putting the information into req.environ. Either returns 404
@@ -197,6 +202,7 @@
197 must, in addition to their normal parameters, accept a 'req' argument202 must, in addition to their normal parameters, accept a 'req' argument
198 which is the incoming webob.Request.203 which is the incoming webob.Request.
199 """204 """
205
200 @webob.dec.wsgify206 @webob.dec.wsgify
201 def __call__(self, req):207 def __call__(self, req):
202 """208 """
@@ -249,6 +255,7 @@
249 return repr(data)255 return repr(data)
250256
251 def _to_xml_node(self, doc, metadata, nodename, data):257 def _to_xml_node(self, doc, metadata, nodename, data):
258 """Recursive method to convert data members to XML nodes."""
252 result = doc.createElement(nodename)259 result = doc.createElement(nodename)
253 if type(data) is list:260 if type(data) is list:
254 singular = metadata.get('plurals', {}).get(nodename, None)261 singular = metadata.get('plurals', {}).get(nodename, None)
@@ -262,7 +269,7 @@
262 result.appendChild(node)269 result.appendChild(node)
263 elif type(data) is dict:270 elif type(data) is dict:
264 attrs = metadata.get('attributes', {}).get(nodename, {})271 attrs = metadata.get('attributes', {}).get(nodename, {})
265 for k,v in data.items():272 for k, v in data.items():
266 if k in attrs:273 if k in attrs:
267 result.setAttribute(k, str(v))274 result.setAttribute(k, str(v))
268 else:275 else:
269276
=== added file 'nova/wsgi_test.py'
--- nova/wsgi_test.py 1970-01-01 00:00:00 +0000
+++ nova/wsgi_test.py 2010-08-19 01:41:08 +0000
@@ -0,0 +1,96 @@
1# vim: tabstop=4 shiftwidth=4 softtabstop=4
2
3# Copyright 2010 United States Government as represented by the
4# Administrator of the National Aeronautics and Space Administration.
5# Copyright 2010 OpenStack LLC.
6# All Rights Reserved.
7#
8# Licensed under the Apache License, Version 2.0 (the "License"); you may
9# not use this file except in compliance with the License. You may obtain
10# a copy of the License at
11#
12# http://www.apache.org/licenses/LICENSE-2.0
13#
14# Unless required by applicable law or agreed to in writing, software
15# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
16# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
17# License for the specific language governing permissions and limitations
18# under the License.
19
20"""
21Test WSGI basics and provide some helper functions for other WSGI tests.
22"""
23
24import unittest
25
26import routes
27import webob
28
29from nova import wsgi
30
31
32class Test(unittest.TestCase):
33
34 def test_debug(self):
35
36 class Application(wsgi.Application):
37 """Dummy application to test debug."""
38
39 def __call__(self, environ, start_response):
40 start_response("200", [("X-Test", "checking")])
41 return ['Test result']
42
43 application = wsgi.Debug(Application())
44 result = webob.Request.blank('/').get_response(application)
45 self.assertEqual(result.body, "Test result")
46
47 def test_router(self):
48
49 class Application(wsgi.Application):
50 """Test application to call from router."""
51
52 def __call__(self, environ, start_response):
53 start_response("200", [])
54 return ['Router result']
55
56 class Router(wsgi.Router):
57 """Test router."""
58
59 def __init__(self):
60 mapper = routes.Mapper()
61 mapper.connect("/test", controller=Application())
62 super(Router, self).__init__(mapper)
63
64 result = webob.Request.blank('/test').get_response(Router())
65 self.assertEqual(result.body, "Router result")
66 result = webob.Request.blank('/bad').get_response(Router())
67 self.assertNotEqual(result.body, "Router result")
68
69 def test_controller(self):
70
71 class Controller(wsgi.Controller):
72 """Test controller to call from router."""
73 test = self
74
75 def show(self, req, id): # pylint: disable-msg=W0622,C0103
76 """Default action called for requests with an ID."""
77 self.test.assertEqual(req.path_info, '/tests/123')
78 self.test.assertEqual(id, '123')
79 return id
80
81 class Router(wsgi.Router):
82 """Test router."""
83
84 def __init__(self):
85 mapper = routes.Mapper()
86 mapper.resource("test", "tests", controller=Controller())
87 super(Router, self).__init__(mapper)
88
89 result = webob.Request.blank('/tests/123').get_response(Router())
90 self.assertEqual(result.body, "123")
91 result = webob.Request.blank('/test/123').get_response(Router())
92 self.assertNotEqual(result.body, "123")
93
94 def test_serializer(self):
95 # TODO(eday): Placeholder for serializer testing.
96 pass
097
=== modified file 'pylintrc'
--- pylintrc 2010-08-18 16:09:29 +0000
+++ pylintrc 2010-08-19 01:41:08 +0000
@@ -1,19 +1,34 @@
1[Messages Control]1[Messages Control]
2<<<<<<< TREE
2disable-msg=C01033disable-msg=C0103
3# TODOs in code comments are fine...4# TODOs in code comments are fine...
4disable-msg=W05115disable-msg=W0511
5# *args and **kwargs are fine6# *args and **kwargs are fine
6disable-msg=W01427disable-msg=W0142
8=======
9# W0511: TODOs in code comments are fine.
10# W0142: *args and **kwargs are fine.
11disable-msg=W0511,W0142
12>>>>>>> MERGE-SOURCE
713
8[Basic]14[Basic]
9# Variables can be 1 to 31 characters long, with15# Variable names can be 1 to 31 characters long, with lowercase and underscores
10# lowercase and underscores
11variable-rgx=[a-z_][a-z0-9_]{0,30}$16variable-rgx=[a-z_][a-z0-9_]{0,30}$
1217
18# Argument names can be 2 to 31 characters long, with lowercase and underscores
19argument-rgx=[a-z_][a-z0-9_]{1,30}$
20
13# Method names should be at least 3 characters long21# Method names should be at least 3 characters long
14# and be lowecased with underscores22# and be lowecased with underscores
15method-rgx=[a-z_][a-z0-9_]{2,50}$23method-rgx=[a-z_][a-z0-9_]{2,50}$
1624
25# Module names matching nova-* are ok (files in bin/)
26module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+)|(nova-[a-z0-9_-]+))$
27
28# Don't require docstrings on tests.
29no-docstring-rgx=((__.*__)|([tT]est.*)|setUp|tearDown)$
30
17[Design]31[Design]
18max-public-methods=10032max-public-methods=100
19min-public-methods=033min-public-methods=0
34max-args=6