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

[Eric Day]

Added unit tests for WSGI helpers and base WSGI API.

Added unit tests for WSGI helpers and base WSGI API.

[Michael Gundlach (gundlach)]

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.

[Eric Day (eday)]

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.

[Eric Day (eday)]

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

[lp:~eday/nova/api-tests]

252. By Eric Day on 2010-08-19

Added '-' as possible charater in module rgx.

253. By Eric Day on 2010-08-19

More pylintrc updates.

[Michael Gundlach (gundlach)]

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

[Michael Gundlach (gundlach)]

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

[Eric Day (eday)]

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.

[Matt Dietz (cerberus)]

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

review: Approve

[Michael Gundlach (gundlach)]

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.

=== 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 @@
 
     def __init__(self):
         mapper = routes.Mapper()
+<<<<<<< TREE
         mapper.connect(None, "/v1.0/{path_info:.*}",
                        controller=rackspace.API())
         mapper.connect(None, "/ec2/{path_info:.*}", controller=ec2.API())
         super(API, self).__init__(mapper)
+=======
+        mapper.connect("/v1.0/{path_info:.*}", controller=rackspace.API())
+        mapper.connect("/ec2/{path_info:.*}", controller=ec2.API())
+        super(API, self).__init__(mapper)
+>>>>>>> MERGE-SOURCE

=== 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 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""
+Test for the root WSGI middleware for all API controllers.
+"""
+
+import unittest
+
+import stubout
+import webob
+import webob.dec
+
+from nova import api
+
+
+class Test(unittest.TestCase):
+
+    def setUp(self): # pylint: disable-msg=C0103
+        self.stubs = stubout.StubOutForTesting()
+
+    def tearDown(self): # pylint: disable-msg=C0103
+        self.stubs.UnsetAll()
+
+    def test_rackspace(self):
+        self.stubs.Set(api.rackspace, 'API', APIStub)
+        result = webob.Request.blank('/v1.0/cloud').get_response(api.API())
+        self.assertEqual(result.body, "/cloud")
+
+    def test_ec2(self):
+        self.stubs.Set(api.ec2, 'API', APIStub)
+        result = webob.Request.blank('/ec2/cloud').get_response(api.API())
+        self.assertEqual(result.body, "/cloud")
+
+    def test_not_found(self):
+        self.stubs.Set(api.ec2, 'API', APIStub)
+        self.stubs.Set(api.rackspace, 'API', APIStub)
+        result = webob.Request.blank('/test/cloud').get_response(api.API())
+        self.assertNotEqual(result.body, "/cloud")
+
+
+class APIStub(object):
+    """Class to verify request and mark it was called."""
+
+    @webob.dec.wsgify
+    def __call__(self, req):
+        return req.path_info

=== 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 @@
         raise NotImplementedError("You must implement __call__")
 
 
+<<<<<<< TREE
 class Middleware(Application): # pylint: disable-msg=W0223
+=======
+class Middleware(Application):
+>>>>>>> MERGE-SOURCE
     """
     Base WSGI middleware wrapper. These classes require an application to be
     initialized that will be called next.  By default the middleware will
@@ -95,7 +99,7 @@
         self.application = application
 
     @webob.dec.wsgify
-    def __call__(self, req):
+    def __call__(self, req): # pylint: disable-msg=W0221
         """Override to implement middleware behavior."""
         return self.application
 
@@ -113,7 +117,7 @@
         resp = req.get_response(self.application)
 
         print ("*" * 40) + " RESPONSE HEADERS"
-        for (key, value) in resp.headers:
+        for (key, value) in resp.headers.iteritems():
             print key, "=", value
         print
 
@@ -127,7 +131,7 @@
         Iterator that prints the contents of a wrapper string iterator
         when iterated.
         """
-        print ("*" * 40) + "BODY"
+        print ("*" * 40) + " BODY"
         for part in app_iter:
             sys.stdout.write(part)
             sys.stdout.flush()
@@ -176,8 +180,9 @@
         """
         return self._router
 
+    @staticmethod
     @webob.dec.wsgify
-    def _dispatch(self, req):
+    def _dispatch(req):
         """
         Called by self._router after matching the incoming request to a route
         and putting the information into req.environ.  Either returns 404
@@ -197,6 +202,7 @@
     must, in addition to their normal parameters, accept a 'req' argument
     which is the incoming webob.Request.
     """
+
     @webob.dec.wsgify
     def __call__(self, req):
         """
@@ -249,6 +255,7 @@
             return repr(data)
 
     def _to_xml_node(self, doc, metadata, nodename, data):
+        """Recursive method to convert data members to XML nodes."""
         result = doc.createElement(nodename)
         if type(data) is list:
             singular = metadata.get('plurals', {}).get(nodename, None)
@@ -262,7 +269,7 @@
                 result.appendChild(node)
         elif type(data) is dict:
             attrs = metadata.get('attributes', {}).get(nodename, {})
-            for k,v in data.items():
+            for k, v in data.items():
                 if k in attrs:
                     result.setAttribute(k, str(v))
                 else:

=== 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 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 United States Government as represented by the
+# Administrator of the National Aeronautics and Space Administration.
+# Copyright 2010 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""
+Test WSGI basics and provide some helper functions for other WSGI tests.
+"""
+
+import unittest
+
+import routes
+import webob
+
+from nova import wsgi
+
+
+class Test(unittest.TestCase):
+
+    def test_debug(self):
+
+        class Application(wsgi.Application):
+            """Dummy application to test debug."""
+
+            def __call__(self, environ, start_response):
+                start_response("200", [("X-Test", "checking")])
+                return ['Test result']
+
+        application = wsgi.Debug(Application())
+        result = webob.Request.blank('/').get_response(application)
+        self.assertEqual(result.body, "Test result")
+
+    def test_router(self):
+
+        class Application(wsgi.Application):
+            """Test application to call from router."""
+
+            def __call__(self, environ, start_response):
+                start_response("200", [])
+                return ['Router result']
+
+        class Router(wsgi.Router):
+            """Test router."""
+
+            def __init__(self):
+                mapper = routes.Mapper()
+                mapper.connect("/test", controller=Application())
+                super(Router, self).__init__(mapper)
+
+        result = webob.Request.blank('/test').get_response(Router())
+        self.assertEqual(result.body, "Router result")
+        result = webob.Request.blank('/bad').get_response(Router())
+        self.assertNotEqual(result.body, "Router result")
+
+    def test_controller(self):
+
+        class Controller(wsgi.Controller):
+            """Test controller to call from router."""
+            test = self
+
+            def show(self, req, id): # pylint: disable-msg=W0622,C0103
+                """Default action called for requests with an ID."""
+                self.test.assertEqual(req.path_info, '/tests/123')
+                self.test.assertEqual(id, '123')
+                return id
+
+        class Router(wsgi.Router):
+            """Test router."""
+
+            def __init__(self):
+                mapper = routes.Mapper()
+                mapper.resource("test", "tests", controller=Controller())
+                super(Router, self).__init__(mapper)
+
+        result = webob.Request.blank('/tests/123').get_response(Router())
+        self.assertEqual(result.body, "123")
+        result = webob.Request.blank('/test/123').get_response(Router())
+        self.assertNotEqual(result.body, "123")
+
+    def test_serializer(self):
+        # TODO(eday): Placeholder for serializer testing.
+        pass

=== modified file 'pylintrc'
--- pylintrc	2010-08-18 16:09:29 +0000
+++ pylintrc	2010-08-19 01:41:08 +0000
@@ -1,19 +1,34 @@
 [Messages Control]
+<<<<<<< TREE
 disable-msg=C0103
 # TODOs in code comments are fine...
 disable-msg=W0511
 # *args and **kwargs are fine
 disable-msg=W0142
+=======
+# W0511: TODOs in code comments are fine.
+# W0142: *args and **kwargs are fine.
+disable-msg=W0511,W0142
+>>>>>>> MERGE-SOURCE
 
 [Basic]
-# Variables can be 1 to 31 characters long, with
-# lowercase and underscores
+# Variable names can be 1 to 31 characters long, with lowercase and underscores
 variable-rgx=[a-z_][a-z0-9_]{0,30}$
 
+# Argument names can be 2 to 31 characters long, with lowercase and underscores
+argument-rgx=[a-z_][a-z0-9_]{1,30}$
+
 # Method names should be at least 3 characters long
 # and be lowecased with underscores
 method-rgx=[a-z_][a-z0-9_]{2,50}$
 
+# Module names matching nova-* are ok (files in bin/)
+module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+)|(nova-[a-z0-9_-]+))$
+
+# Don't require docstrings on tests.
+no-docstring-rgx=((__.*__)|([tT]est.*)|setUp|tearDown)$
+
 [Design]
 max-public-methods=100
 min-public-methods=0
+max-args=6