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>

diff --git a/lib/rapi/baserlib.py b/lib/rapi/baserlib.py
index 0edd96b..44da78c 100644 (file)
--- 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 00c9678..c971577 100755 (executable)
--- 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 e19e800..374f1fe 100755 (executable)
--- 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()