Merge pull request #57 from gregoil/fix_kwargs_metaclass

Fix kwargs passing to BaseResource

src/rotest/management/base_resource.py

@@ -16,18 +16,28 @@
 
 
 class ConvertToKwargsMeta(type):
-    """Metaclass that converts args to kwargs when creating an instance.
+    """Metaclass that validates no positional args are passed to constructor.
 
-    This enables requesting resources that have args more easily, e.g.:
-        Assuming ResClass gets 'x' in the __init__, then without this meta
-        you'd have to request the resource like this:
-            res1 = ResClass(x=5)
-        but with the meta you can request it like this:
+    This enables avoiding requesting resources is coherent, i.e.:
+        Assuming ResClass gets 'x' in the __init__, then when requesting:
             res1 = ResClass(5)
+        The x=5 is not passed to kwargs, thus is not propagated when
+        creating the actual resource when the test starts.
     """
     def __call__(cls, *args, **kwargs):
-        kwargs.update(zip(cls.__init__.func_code.co_varnames[1:], args))
-        return type.__call__(cls, **kwargs)
+        if len(args) > 0:
+            raise RuntimeError("BaseResource constructors must not get "
+                               "positional arguments")
+
+        resource = type.__call__(cls, *args, **kwargs)
+        resource.kwargs = kwargs
+        for field_name, field_value in kwargs.iteritems():
+            setattr(resource, field_name, field_value)
+
+        if isinstance(resource.data, AttrDict):
+            resource.data.update(kwargs)
+
+        return resource
 
 
 class BaseResource(object):
@@ -70,7 +80,7 @@ def __init__(self, data=None, **kwargs):
                 setattr(self, field_name, field_value)
 
         else:
-            self.data = AttrDict(kwargs)
+            self.data = AttrDict()
             self.name = "%s-%d" % (self.__class__.__name__, id(self))
 
         self.config = None
@@ -81,9 +91,6 @@ def __init__(self, data=None, **kwargs):
 
         self._sub_resources = None
 
-        for field_name, field_value in kwargs.iteritems():
-            setattr(self, field_name, field_value)
-
     def create_sub_resources(self):
         """Create and return the sub resources if needed.
 

src/rotest/management/models/ut_models.py

@@ -169,8 +169,8 @@ class DemoComplexResource(BaseResource):
 
     def create_sub_resources(self):
         """Return an iterable to the complex resource's sub-resources."""
-        self.demo1 = DemoResource(self.data.demo1)
-        self.demo2 = DemoResource(self.data.demo2)
+        self.demo1 = DemoResource(data=self.data.demo1)
+        self.demo2 = DemoResource(data=self.data.demo2)
         return (self.demo1, self.demo2)
 
     def initialize(self):

tests/core/utils.py

@@ -241,7 +241,7 @@ def _lock_resources(self, descriptors, timeout=None):
                     if len(available_resources) == 0:
                         raise ResourceDoesNotExistError()
 
-                    resource = descriptor.type(available_resources[0])
+                    resource = descriptor.type(data=available_resources[0])
 
                 except ObjectDoesNotExist:  # The resource doesn't exist.
                     raise ResourceDoesNotExistError()

tests/management/test_resource_manager.py

@@ -571,7 +571,7 @@ def test_lock_release_complex_resource(self):
                              "Sub-resource %r should be locked but "
                              "found available" % sub_resource.name)
 
-        resource_instace = DemoResource(resources.get())
+        resource_instace = DemoResource(data=resources.get())
         self.client._release_resources(resources=[resource_instace])
 
         resources = DemoComplexResourceData.objects.filter(
@@ -624,7 +624,7 @@ def test_release_free_resource(self):
         * Validates a ResourceAlreadyAvailableError is raised.
         """
         resources = DemoResource(
-                         self.get_resource(self.FREE1_NAME, owner="").get())
+            data=self.get_resource(self.FREE1_NAME, owner="").get())
 
         with self.assertRaises(ResourceReleaseError) as cm:
             self.client._release_resources(resources=[resources])
@@ -660,7 +660,7 @@ def test_release_non_existing_resource(self):
                          % (self.NON_EXISTING_NAME1, resources_num))
 
         non_existing_resource = DemoResource(
-                             DemoResourceData(name=self.NON_EXISTING_NAME1))
+            data=DemoResourceData(name=self.NON_EXISTING_NAME1))
 
         with self.assertRaises(ResourceReleaseError) as cm:
             self.client._release_resources(resources=[non_existing_resource])
@@ -692,7 +692,7 @@ def test_release_resources_multiple_failures(self):
             resource.
         """
         resources = DemoResource(
-                         self.get_resource(self.FREE1_NAME, owner="").get())
+            data=self.get_resource(self.FREE1_NAME, owner="").get())
 
         resources_num = DemoResourceData.objects.filter(
                                         name=self.NON_EXISTING_NAME1).count()
@@ -702,7 +702,7 @@ def test_release_resources_multiple_failures(self):
                          % (self.NON_EXISTING_NAME1, resources_num))
 
         non_existing_resource = DemoResource(
-                             DemoResourceData(name=self.NON_EXISTING_NAME1))
+            data=DemoResourceData(name=self.NON_EXISTING_NAME1))
 
         resources = [resources] + [non_existing_resource]
 
@@ -747,7 +747,7 @@ def test_one_lock_other_release(self):
                          "resource with name %r in DB, found %d"
                          % (self.LOCKED1_NAME, resources_num))
 
-        resource = DemoResource(resources.get())
+        resource = DemoResource(data=resources.get())
         with self.assertRaises(ResourceReleaseError) as cm:
             self.client._release_resources(resources=[resource])
 

tests/management/test_result_manager.py

@@ -170,7 +170,7 @@ def test_update_resources(self):
         test_case = next(iter(main_test))
 
         test_case.locked_resources = {'test_resource': DemoResource(
-                  DemoResourceData.objects.get(name='available_resource1'))}
+            data=DemoResourceData.objects.get(name='available_resource1'))}
 
         self.client.start_test_run(main_test)
         self.client.start_test(test_case)