From a147fa3350dd2402c13405a0da1c47f419f53ced Mon Sep 17 00:00:00 2001
From: Sergei Trofimov <sergei.trofimov@arm.com>
Date: Wed, 26 Apr 2017 16:09:36 +0100
Subject: [PATCH] Fix Plugin and tests

WA2 Extensions had two features that have proven more trouble then they
are worth, and so they were removed from WA3 Plugins:

- Virtual methods (methods that automatically invoked super's version
  without having to explicitly do that).
- It used to be possible to use strings or tuples to specify parameters
  for extensions, and those were automatically "promoted" to Parameter
  instances by the metaclass. This feature was never actually used.

This commit cleans up residual code for these features from Plugin
implementations and removes tests that used to exercise them.
---
 wa/framework/plugin.py                        | 30 ++++---------
 .../data/extensions/devices/test_device.py    |  1 -
 wa/tests/test_plugin.py                       | 44 -------------------
 3 files changed, 8 insertions(+), 67 deletions(-)

diff --git a/wa/framework/plugin.py b/wa/framework/plugin.py
index c9828839..bc7ab832 100644
--- a/wa/framework/plugin.py
+++ b/wa/framework/plugin.py
@@ -81,19 +81,10 @@ class AttributeCollection(object):
     __repr__ = __str__
 
     def _to_attrcls(self, p):
-        old_owner = getattr(p, "_owner", None)
-        if isinstance(p, basestring):
-            p = self._attrcls(p)
-        elif isinstance(p, tuple) or isinstance(p, list):
-            p = self._attrcls(*p)
-        elif isinstance(p, dict):
-            p = self._attrcls(**p)
-        elif not isinstance(p, self._attrcls):
-            raise ValueError('Invalid parameter value: {}'.format(p))
-        if (p.name in self._attrs and not p.override and
-                p.name != 'modules'):  # TODO: HACK due to "diamond dependecy" in workloads...
+        if not isinstance(p, self._attrcls):
+            raise ValueError('Invalid attribute value: {}; must be a {}'.format(p, self._attrcls))
+        if (p.name in self._attrs and not p.override):
             raise ValueError('Attribute {} has already been defined.'.format(p.name))
-        p._owner = old_owner
         return p
 
     def __iadd__(self, other):
@@ -269,7 +260,6 @@ class PluginMeta(type):
     to_propagate = [
         ('parameters', Parameter, AttributeCollection),
         ('artifacts', Artifact, AttributeCollection),
-        ('core_modules', str, ListCollection),
     ]
 
     virtual_methods = ['validate', 'initialize', 'finalize']
@@ -299,8 +289,10 @@ class PluginMeta(type):
             if prop_attr in attrs:
                 pattrs = attrs[prop_attr] or []
                 for pa in pattrs:
-                    if not isinstance(pa, basestring):
-                        pa._owner = clsname
+                    if not isinstance(pa, attr_cls):
+                        msg = 'Invalid value "{}" for attribute "{}"; must be a {}'
+                        raise ValueError(msg.format(pa, prop_attr, attr_cls))
+                    pa._owner = clsname
                 propagated += pattrs
                 should_propagate = True
             if should_propagate:
@@ -339,13 +331,7 @@ class Plugin(object):
 
     kind = None
     name = None
-    parameters = [
-        Parameter('modules', kind=list,
-                  description="""
-                  Lists the modules to be loaded by this plugin. A module is a 
-                  plug-in that further extends functionality of an plugin.
-                  """),
-    ]
+    parameters = []
     artifacts = []
     aliases = []
     core_modules = []
diff --git a/wa/tests/data/extensions/devices/test_device.py b/wa/tests/data/extensions/devices/test_device.py
index 2c4d51ad..e2c9d278 100644
--- a/wa/tests/data/extensions/devices/test_device.py
+++ b/wa/tests/data/extensions/devices/test_device.py
@@ -23,7 +23,6 @@ class TestDevice(Plugin):
     kind = 'device'
 
     def __init__(self, *args, **kwargs):
-        self.modules = []
         self.boot_called = 0
         self.push_file_called = 0
         self.pull_file_called = 0
diff --git a/wa/tests/test_plugin.py b/wa/tests/test_plugin.py
index f47f488a..a8bc5c24 100644
--- a/wa/tests/test_plugin.py
+++ b/wa/tests/test_plugin.py
@@ -44,15 +44,8 @@ class PluginLoaderTest(TestCase):
 
 
 
-class MyMeta(PluginMeta):
-
-    virtual_methods = ['validate', 'virtual1', 'virtual2']
-
-
 class MyBasePlugin(Plugin):
 
-    __metaclass__ = MyMeta
-
     name = 'base'
     kind = 'test'
 
@@ -166,43 +159,6 @@ class PluginMetaTest(TestCase):
                 7,
             ]
 
-    def test_virtual_methods(self):
-        acid = MyAcidPlugin()
-        acid.virtual1()
-        assert_equal(acid.v1, 1)
-        assert_equal(acid.vv1, 1)
-        assert_equal(acid.v2, 0)
-        assert_equal(acid.vv2, 0)
-        assert_equal(acid.v3, 'acid')
-        acid.virtual2()
-        acid.virtual2()
-        assert_equal(acid.v1, 1)
-        assert_equal(acid.vv1, 1)
-        assert_equal(acid.v2, 2)
-        assert_equal(acid.vv2, 2)
-
-    def test_initialization(self):
-        class MyExt(Plugin):
-            name = 'myext'
-            kind = 'test'
-            values = {'a': 0}
-            def __init__(self, *args, **kwargs):
-                super(MyExt, self).__init__(*args, **kwargs)
-                self.instance_init = 0
-            def initialize(self, context):
-                self.values['a'] += 1
-
-        class MyChildExt(MyExt):
-            name = 'mychildext'
-            def initialize(self, context):
-                self.instance_init += 1
-
-        ext = MyChildExt()
-        ext.initialize(None)
-
-        assert_equal(MyExt.values['a'], 1)
-        assert_equal(ext.instance_init, 1)
-
 
 class ParametersTest(TestCase):