From 61f8fda42d732f7e7326bcbc2f3e03b0967832d2 Mon Sep 17 00:00:00 2001
From: Michele Tartara <mtartara@google.com>
Date: Thu, 21 Mar 2013 16:54:43 +0100
Subject: [PATCH] Fix bug in rlib2 unit tests

The "queryargs" parameter of the __init__ function of rlib2 classes should
be a dictionary, as defined in the ResourceBase parent class (in
lib/rapi/baserlib.py).

In the rlib2 and baserlib unit tests, when the queryargs parameter is not
actually needed, it is sometimes erroneously initialized with an empty list or a
"None" value instead of an empty dictionary.

This commit solves the problem and introduces an assertion to prevent it from
happening again in the future.
The use of the assertion is safe, because in production code the __init__
function is only called by the RAPI server, with the queryargs parameter
initialized by the Mapper.getController function (lib/rapi/connector.py) that
always returns a dictionary. It can only affect test code and future code.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>
---
 lib/rapi/baserlib.py                     |  2 ++
 test/py/ganeti.rapi.baserlib_unittest.py |  8 ++++----
 test/py/ganeti.rapi.rlib2_unittest.py    | 10 +++++-----
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/lib/rapi/baserlib.py b/lib/rapi/baserlib.py
index 0edd96b85..44da78c1d 100644
--- a/lib/rapi/baserlib.py
+++ b/lib/rapi/baserlib.py
@@ -263,6 +263,8 @@ class ResourceBase(object):
     @param _client_cls: L{luxi} client class (unittests only)
 
     """
+    assert isinstance(queryargs, dict)
+
     self.items = items
     self.queryargs = queryargs
     self._req = req
diff --git a/test/py/ganeti.rapi.baserlib_unittest.py b/test/py/ganeti.rapi.baserlib_unittest.py
index 00c9678a3..c9715775c 100755
--- a/test/py/ganeti.rapi.baserlib_unittest.py
+++ b/test/py/ganeti.rapi.baserlib_unittest.py
@@ -115,14 +115,14 @@ class TestOpcodeResource(unittest.TestCase):
   def test(self):
     for method in baserlib._SUPPORTED_METHODS:
       # Empty handler
-      obj = self._MakeClass(method, {})(None, None, None)
+      obj = self._MakeClass(method, {})(None, {}, None)
       for attr in itertools.chain(*baserlib.OPCODE_ATTRS):
         self.assertFalse(hasattr(obj, attr))
 
       # Direct handler function
       obj = self._MakeClass(method, {
         method: lambda _: None,
-        })(None, None, None)
+        })(None, {}, None)
       self.assertFalse(compat.all(hasattr(obj, attr)
                                   for i in baserlib._SUPPORTED_METHODS
                                   for attr in self._GetMethodAttributes(i)))
@@ -131,7 +131,7 @@ class TestOpcodeResource(unittest.TestCase):
       for opcls in [None, object()]:
         obj = self._MakeClass(method, {
           "%s_OPCODE" % method: opcls,
-          })(None, None, None)
+          })(None, {}, None)
         self.assertTrue(callable(getattr(obj, method)))
         self.assertEqual(getattr(obj, "%s_OPCODE" % method), opcls)
         self.assertFalse(hasattr(obj, "%s_RENAME" % method))
@@ -151,7 +151,7 @@ class TestOpcodeResource(unittest.TestCase):
     class _Empty(baserlib.OpcodeResource):
       pass
 
-    obj = _Empty(None, None, None)
+    obj = _Empty(None, {}, None)
 
     for attr in itertools.chain(*baserlib.OPCODE_ATTRS):
       self.assertFalse(hasattr(obj, attr))
diff --git a/test/py/ganeti.rapi.rlib2_unittest.py b/test/py/ganeti.rapi.rlib2_unittest.py
index e19e800f0..374f1feb7 100755
--- a/test/py/ganeti.rapi.rlib2_unittest.py
+++ b/test/py/ganeti.rapi.rlib2_unittest.py
@@ -116,7 +116,7 @@ class TestClientConnectError(unittest.TestCase):
       rlib2.R_2_nodes,
       ]
     for cls in resources:
-      handler = _CreateHandler(cls, ["name"], [], None, self._FailingClient)
+      handler = _CreateHandler(cls, ["name"], {}, None, self._FailingClient)
       self.assertRaises(http.HttpBadGateway, handler.GET)
 
 
@@ -130,7 +130,7 @@ class TestJobSubmitError(unittest.TestCase):
       raise errors.JobQueueFull("test")
 
   def test(self):
-    handler = _CreateHandler(rlib2.R_2_redist_config, [], [], None,
+    handler = _CreateHandler(rlib2.R_2_redist_config, [], {}, None,
                              self._SubmitErrorClient)
     self.assertRaises(http.HttpServiceUnavailable, handler.PUT)
 
@@ -138,7 +138,7 @@ class TestJobSubmitError(unittest.TestCase):
 class TestClusterModify(unittest.TestCase):
   def test(self):
     clfactory = _FakeClientFactory(_FakeClient)
-    handler = _CreateHandler(rlib2.R_2_cluster_modify, [], [], {
+    handler = _CreateHandler(rlib2.R_2_cluster_modify, [], {}, {
       "vg_name": "testvg",
       "candidate_pool_size": 100,
       }, clfactory)
@@ -158,7 +158,7 @@ class TestClusterModify(unittest.TestCase):
   def testInvalidValue(self):
     for attr in ["vg_name", "candidate_pool_size", "beparams", "_-Unknown#"]:
       clfactory = _FakeClientFactory(_FakeClient)
-      handler = _CreateHandler(rlib2.R_2_cluster_modify, [], [], {
+      handler = _CreateHandler(rlib2.R_2_cluster_modify, [], {}, {
         attr: True,
         }, clfactory)
       self.assertRaises(http.HttpBadRequest, handler.PUT)
@@ -168,7 +168,7 @@ class TestClusterModify(unittest.TestCase):
 class TestRedistConfig(unittest.TestCase):
   def test(self):
     clfactory = _FakeClientFactory(_FakeClient)
-    handler = _CreateHandler(rlib2.R_2_redist_config, [], [], None, clfactory)
+    handler = _CreateHandler(rlib2.R_2_redist_config, [], {}, None, clfactory)
     job_id = handler.PUT()
 
     cl = clfactory.GetNextClient()
-- 
GitLab