Merge branch 'ldoktor-mux-filters3'

Signed-off-by: NAmador Pahim <apahim@redhat.com>

avocado/core/dispatcher.py

@@ -21,6 +21,7 @@ import sys
 from stevedore import EnabledExtensionManager
 
 from .settings import settings
+from ..utils import stacktrace
 
 
 class Dispatcher(EnabledExtensionManager):
@@ -242,6 +243,7 @@ class VarianterDispatcher(Dispatcher):
             except KeyboardInterrupt:
                 raise
             except:     # catch any exception pylint: disable=W0702
+                stacktrace.log_exc_info(sys.exc_info(), logger='avocado.debug')
                 log = logging.getLogger("avocado.app")
                 log.error('Error running method "%s" of plugin "%s": %s',
                           method_name, ext.name, sys.exc_info()[1])

avocado/core/mux.py

@@ -71,19 +71,74 @@ class MuxTree(object):
 
     def __iter__(self):
         """
-        Iterates through variants
+        Iterates through variants and process the internal filters
+
+        :yield valid variants
+        """
+        for variant in self.iter_variants():
+            if self._valid_variant(variant):
+                yield variant
+
+    def iter_variants(self):
+        """
+        Iterates through variants without verifying the internal filters
+
+        :yield all existing variants
         """
         pools = []
         for pool in self.pools:
             if isinstance(pool, list):
-                pools.append(itertools.chain(*pool))
+                # Don't process 2nd level filters in non-root pools
+                pools.append(itertools.chain(*(_.iter_variants()
+                                               for _ in pool)))
             else:
-                pools.append(pool)
-        pools = itertools.product(*pools)
+                pools.append([pool])
+        variants = itertools.product(*pools)
         while True:
-            # TODO: Implement 2nd level filters here
-            # TODO: This part takes most of the time, optimize it
-            yield list(itertools.chain(*pools.next()))
+            yield list(itertools.chain(*variants.next()))
+
+    @staticmethod
+    def _valid_variant(variant):
+        """
+        Check the variant for validity of internal filters
+
+        :return: whether the variant is valid or should be ignored/filtered
+        """
+        _filter_out = set()
+        _filter_only = set()
+        for node in variant:
+            _filter_only.update(node.environment.filter_only)
+            _filter_out.update(node.environment.filter_out)
+        if not (_filter_only or _filter_out):
+            return True
+        filter_only = tuple(_filter_only)
+        filter_out = tuple(_filter_out)
+        filter_only_parents = [str(_).rsplit('/', 2)[0] + '/'
+                               for _ in filter_only
+                               if _]
+
+        for out in filter_out:
+            for node in variant:
+                path = node.path + '/'
+                if path.startswith(out):
+                    return False
+        for node in variant:
+            keep = 0
+            remove = 0
+            path = node.path + '/'
+            ppath = path.rsplit('/', 2)[0] + '/'
+            for i in xrange(len(filter_only)):
+                level = filter_only[i].count('/')
+                if level < max(keep, remove):
+                    continue
+                if ppath.startswith(filter_only_parents[i]):
+                    if path.startswith(filter_only[i]):
+                        keep = level
+                    else:
+                        remove = level
+            if remove > keep:
+                return False
+        return True
 
 
 class MuxPlugin(object):
@@ -172,7 +227,7 @@ class MuxPlugin(object):
                     env = set()
                     for node in variant["variant"]:
                         for key, value in node.environment.iteritems():
-                            origin = node.environment_origin[key].path
+                            origin = node.environment.origin[key].path
                             env.add(("%s:%s" % (origin, key), str(value)))
                     if not env:
                         continue

avocado/core/tree.py

@@ -41,6 +41,48 @@ import os
 from . import output
 
 
+class FilterSet(set):
+
+    """ Set of filters in standardized form """
+
+    @staticmethod
+    def __normalize(item):
+        if not item.endswith("/"):
+            item = item + "/"
+        return item
+
+    def add(self, item):
+        return super(FilterSet, self).add(self.__normalize(item))
+
+    def update(self, items):
+        return super(FilterSet, self).update([self.__normalize(item)
+                                              for item in items])
+
+
+class TreeEnvironment(dict):
+
+    """ TreeNode environment with values, origins and filters """
+
+    def __init__(self):
+        super(TreeEnvironment, self).__init__()     # values
+        self.origin = {}    # origins of the values
+        self.filter_only = FilterSet()   # list of filter_only
+        self.filter_out = FilterSet()    # list of filter_out
+
+    def copy(self):
+        cpy = TreeEnvironment()
+        cpy.update(self)
+        cpy.origin = self.origin.copy()
+        cpy.filter_only = self.filter_only.copy()
+        cpy.filter_out = self.filter_out.copy()
+        return cpy
+
+    def __str__(self):
+        return ",".join((super(TreeEnvironment, self).__str__(),
+                         str(self.origin), str(self.filter_only),
+                         str(self.filter_out)))
+
+
 class TreeNode(object):
 
     """
@@ -54,10 +96,10 @@ class TreeNode(object):
             children = []
         self.name = name
         self.value = value
+        self.filters = [], []  # This node filters, full filters in environ..
         self.parent = parent
         self.children = []
         self._environment = None
-        self.environment_origin = {}
         for child in children:
             self.add_child(child)
 
@@ -114,6 +156,8 @@ class TreeNode(object):
         or merged into existing node in the previous position.
         """
         self.value.update(other.value)
+        self.filters[0].extend(other.filters[0])
+        self.filters[1].extend(other.filters[1])
         for child in other.children:
             self.add_child(child)
 
@@ -175,9 +219,7 @@ class TreeNode(object):
         """ Get node environment (values + preceding envs) """
         if self._environment is None:
             self._environment = (self.parent.environment.copy()
-                                 if self.parent else {})
-            self.environment_origin = (self.parent.environment_origin.copy()
-                                       if self.parent else {})
+                                 if self.parent else TreeEnvironment())
             for key, value in self.value.iteritems():
                 if isinstance(value, list):
                     if (key in self._environment and
@@ -187,7 +229,9 @@ class TreeNode(object):
                         self._environment[key] = value
                 else:
                     self._environment[key] = value
-                self.environment_origin[key] = self
+                self._environment.origin[key] = self
+            self._environment.filter_only.update(self.filters[0])
+            self._environment.filter_out.update(self.filters[1])
         return self._environment
 
     def set_environment_dirty(self):
@@ -425,9 +469,17 @@ def tree_view(root, verbose=None, use_utf8=None):
             right = charset['Right']
         out = [node.name]
         if verbose >= 2 and node.is_leaf:
-            values = node.environment.iteritems()
+            values = itertools.chain(node.environment.iteritems(),
+                                     [("filter-only", _)
+                                      for _ in node.environment.filter_only],
+                                     [("filter-out", _)
+                                      for _ in node.environment.filter_out])
         elif verbose in (1, 3):
-            values = node.value.iteritems()
+            values = itertools.chain(node.value.iteritems(),
+                                     [("filter-only", _)
+                                      for _ in node.filters[0]],
+                                     [("filter-out", _)
+                                      for _ in node.filters[1]])
         else:
             values = None
         if values:

avocado/core/varianter.py

@@ -289,7 +289,7 @@ class AvocadoParam(object):
         :raise KeyError: When value is not certain (multiple matches)
         """
         leaves = self._get_leaves(path)
-        ret = [(leaf.environment[key], leaf.environment_origin[key])
+        ret = [(leaf.environment[key], leaf.environment.origin[key])
                for leaf in leaves
                if key in leaf.environment]
         if not ret:
@@ -310,7 +310,7 @@ class AvocadoParam(object):
         """
         for leaf in self._leaves:
             for key, value in leaf.environment.iteritems():
-                yield (leaf.environment_origin[key].path, key, value)
+                yield (leaf.environment.origin[key].path, key, value)
 
 
 class Varianter(object):

avocado/plugins/yaml_to_mux.py

@@ -41,6 +41,8 @@ YAML_USING = 101
 YAML_REMOVE_NODE = mux.REMOVE_NODE
 YAML_REMOVE_VALUE = mux.REMOVE_VALUE
 YAML_MUX = 102
+YAML_FILTER_ONLY = 103
+YAML_FILTER_OUT = 104
 
 __RE_FILE_SPLIT = re.compile(r'(?<!\\):')   # split by ':' but not '\\:'
 __RE_FILE_SUBS = re.compile(r'(?<!\\)\\:')  # substitute '\\:' but not '\\\\:'
@@ -50,14 +52,18 @@ class _BaseLoader(Loader):
     """
     YAML loader with additional features related to mux
     """
-    Loader.add_constructor(u'!include', lambda loader,
-                           node: mux.Control(YAML_INCLUDE))
+    Loader.add_constructor(u'!include',
+                           lambda loader, node: mux.Control(YAML_INCLUDE))
     Loader.add_constructor(u'!using',
                            lambda loader, node: mux.Control(YAML_USING))
     Loader.add_constructor(u'!remove_node',
                            lambda loader, node: mux.Control(YAML_REMOVE_NODE))
     Loader.add_constructor(u'!remove_value',
                            lambda loader, node: mux.Control(YAML_REMOVE_VALUE))
+    Loader.add_constructor(u'!filter-only',
+                           lambda loader, node: mux.Control(YAML_FILTER_ONLY))
+    Loader.add_constructor(u'!filter-out',
+                           lambda loader, node: mux.Control(YAML_FILTER_OUT))
 
 
 class Value(tuple):     # Few methods pylint: disable=R0903
@@ -78,38 +84,64 @@ def _create_from_yaml(path, cls_node=mux.MuxTreeNode):
     """ Create tree structure from yaml stream """
     def tree_node_from_values(name, values):
         """ Create `name` node and add values  """
+        def handle_control_tag(node, value):
+            """ Handling of YAML tags (except of !using) """
+            def normalize_path(path):
+                """ End the path with single '/', None when empty path """
+                if not path:
+                    return
+                if path[-1] != '/':
+                    path += '/'
+                return path
+
+            if value[0].code == YAML_INCLUDE:
+                # Include file
+                ypath = value[1]
+                if not os.path.isabs(ypath):
+                    ypath = os.path.join(os.path.dirname(path), ypath)
+                if not os.path.exists(ypath):
+                    raise ValueError("File '%s' included from '%s' does not "
+                                     "exist." % (ypath, path))
+                node.merge(_create_from_yaml('/:' + ypath, cls_node))
+            elif value[0].code == YAML_REMOVE_NODE:
+                value[0].value = value[1]   # set the name
+                node.ctrl.append(value[0])    # add "blue pill" of death
+            elif value[0].code == YAML_REMOVE_VALUE:
+                value[0].value = value[1]   # set the name
+                node.ctrl.append(value[0])
+            elif value[0].code == YAML_MUX:
+                node.multiplex = True
+            elif value[0].code == YAML_FILTER_ONLY:
+                new_value = normalize_path(value[1])
+                if new_value:
+                    node.filters[0].append(new_value)
+            elif value[0].code == YAML_FILTER_OUT:
+                new_value = normalize_path(value[1])
+                if new_value:
+                    node.filters[1].append(new_value)
+
+        def handle_control_tag_using(name, using, value):
+            """ Handling of the !using tag """
+            if using:
+                raise ValueError("!using can be used only once per "
+                                 "node! (%s:%s)" % (path, name))
+            using = value[1]
+            if using[0] == '/':
+                using = using[1:]
+            if using[-1] == '/':
+                using = using[:-1]
+            return using
+
         node = cls_node(str(name))
         using = ''
         for value in values:
             if isinstance(value, cls_node):
                 node.add_child(value)
             elif isinstance(value[0], mux.Control):
-                if value[0].code == YAML_INCLUDE:
-                    # Include file
-                    ypath = value[1]
-                    if not os.path.isabs(ypath):
-                        ypath = os.path.join(os.path.dirname(path), ypath)
-                    if not os.path.exists(ypath):
-                        raise ValueError("File '%s' included from '%s' does not "
-                                         "exist." % (ypath, path))
-                    node.merge(_create_from_yaml('/:' + ypath, cls_node))
-                elif value[0].code == YAML_USING:
-                    if using:
-                        raise ValueError("!using can be used only once per "
-                                         "node! (%s:%s)" % (path, name))
-                    using = value[1]
-                    if using[0] == '/':
-                        using = using[1:]
-                    if using[-1] == '/':
-                        using = using[:-1]
-                elif value[0].code == YAML_REMOVE_NODE:
-                    value[0].value = value[1]   # set the name
-                    node.ctrl.append(value[0])    # add "blue pill" of death
-                elif value[0].code == YAML_REMOVE_VALUE:
-                    value[0].value = value[1]   # set the name
-                    node.ctrl.append(value[0])
-                elif value[0].code == YAML_MUX:
-                    node.multiplex = True
+                if value[0].code == YAML_USING:
+                    using = handle_control_tag_using(name, using, value)
+                else:
+                    handle_control_tag(node, value)
             else:
                 node.value[value[0]] = value[1]
         if using:

docs/source/TestParameters.rst

@@ -867,6 +867,45 @@ Children of this node will be multiplexed. This means that in first variant
 it'll return leaves of the first child, in second the leaves of the second
 child, etc. Example is in section `Variants`_
 
+!filter-only
+------------
+
+Defines internal filters. They are inherited by children and evaluated
+during multiplexation. It allows one to specify the only compatible branch
+of the tree with the current variant, for example::
+
+    cpu:
+        arm:
+            !filter-only : /disk/virtio
+    disk:
+        virtio:
+        scsi:
+
+will skip the ``[arm, scsi]`` variant and result only in ``[arm, virtio]``
+
+_Note: It's possible to use ``!filter-only`` multiple times with the same
+parent and all allowed variants will be included (unless they are
+filtered-out by ``!filter-out``)_
+
+_Note2: The evaluation order is 1. filter-out, 2. filter-only. This means when
+you booth filter-out and filter-only a branch it won't take part in the
+multiplexed variants._
+
+!filter-out
+-----------
+
+Similarly to `!filter-only`_ only it skips the specified branches and leaves
+the remaining ones. (in the same example the use of
+``!filter-out : /disk/scsi`` results in the same behavior). The difference
+is when a new disk type is introduced, ``!filter-only`` still allows just
+the specified variants, while ``!filter-out`` only removes the specified
+ones.
+
+As for the speed optimization, currently Avocado is strongly optimized
+towards fast ``!filter-out`` so it's highly recommended using them
+rather than ``!filter-only``, which takes significantly longer to
+process.
+
 Complete example
 ----------------
 

examples/mux-selftest.yaml

@@ -7,16 +7,32 @@
 #          multiple files and checks that the node ordering works fine.
 # /env/opt_CFLAGS: Should be present in merged node
 # /env/prod/opt_CFLAGS: value should be overridden by latter node
+# The internal filters are designed to be used for this file injected into
+# /virt (use -m /virt:examples/mux-selftest.py). When it's injected into
+# a different location those filters should not affect the result (produces
+# all variants.
+# !filter-only: All root childern are specified in different levels. They
+#               should be combined and together enable all variants. On the
+#               other hand they should not enable other-level filter-only
+#               filters like /hw/disk/virtio.
+
 hw:
+    # This filter has no effect, it's here to test filter inheritance
+    !filter-out : /this/does/not/exists
     cpu: !mux
+        # This filter has no effect, it's here to test filter inheritance
+        !filter-out : /non/existing/node
         joinlist:
             - first_item
         intel:
+            !filter-only : "/virt/hw/disk/virtio"
+            !filter-only : "/virt/hw/disk/scsi"
             cpu_CFLAGS: '-march=core2'
         amd:
             joinlist: ['second', 'third']
             cpu_CFLAGS: '-march=athlon64'
         arm:
+            !filter-only : "/virt/hw/disk/virtio"
             cpu_CFLAGS: '-mabi=apcs-gnu -march=armv8-a -mtune=arm8'
     disk: !mux
         disk_type: 'virtio'
@@ -28,15 +44,16 @@ hw:
     corruptlist: ['upper_node_list']
 distro: !mux     # This node is set as !multiplex below
     fedora:
+        !filter-out : "/virt/hw/disk/scsi"
         init: 'systemd'
 env: !mux
     opt_CFLAGS: '-Os'
     prod:
         opt_CFLAGS: 'THIS SHOULD GET OVERWRITTEN'
 env: !mux
+    !filter-out : "/yet/another/nonexisting/node"  # let's see if filters are updated when merging
     prod:
         opt_CFLAGS: '-O2'
 distro: !mux
     mint:
         init: 'systemv'
-

selftests/unit/test_mux.py

@@ -9,7 +9,7 @@ from avocado.plugins import yaml_to_mux
 
 
 if __name__ == "__main__":
-    PATH_PREFIX = "../../../../"
+    PATH_PREFIX = "../../"
 else:
     PATH_PREFIX = ""
 
@@ -406,6 +406,78 @@ class TestMultipleLoaders(unittest.TestCase):
         self.assertEqual(type(plain), dict)
 
 
+class TestInternalFilters(unittest.TestCase):
+
+    def check_scenario(self, *args):
+        """
+        Turn args into scenario.
+
+        :param *args: Definitions of variant's nodes. Each arg has to be of
+                      length 3, where on index:
+                        [0] is path
+                        [1] is filter-only
+                        [2] is filter-out
+        """
+        variant = []
+        # Turn scenario into variant
+        for arg in args:
+            variant.append(tree.TreeNode().get_node(arg[0], True))
+            variant[-1].filters = [arg[1], arg[2]]
+        # Check directly the MuxTree._valid_variant function
+        return mux.MuxTree._valid_variant(variant)  # pylint: disable=W0212
+
+    def test_basic(self):
+        """
+        Check basic internal filters
+        """
+        self.assertTrue(self.check_scenario())
+        self.assertTrue(self.check_scenario(("foo", [], []),))
+        self.assertTrue(self.check_scenario(("foo", ["/foo"], []),))
+        self.assertFalse(self.check_scenario(("foo", [], ["/foo"]),))
+        # Filter should be normalized automatically (tailing '/')
+        self.assertTrue(self.check_scenario(("foo", ["/foo/"], []),))
+        self.assertFalse(self.check_scenario(("foo", [], ["/foo/"]),))
+        # Filter-out nonexistings
+        self.assertTrue(self.check_scenario(("foo", [], ["/nonexist"]),))
+        self.assertTrue(self.check_scenario(("foo", [], []),
+                                            ("bar", [], ["/nonexists"])))
+        self.assertTrue(self.check_scenario(("1/foo", [], []),
+                                            ("1/bar", ["/1"], [])))
+        # The /1/foo is not the same parent as /2/bar filter
+        self.assertTrue(self.check_scenario(("1/foo", [], []),
+                                            ("2/bar", ["/2/bar"], [])))
+        self.assertFalse(self.check_scenario(("/1/foo", ["/1/bar"], []),))
+        # Even though it matches one of the leaves the other is banned
+        self.assertFalse(self.check_scenario(("1/foo", ["/1/foo"], []),
+                                             ("1/bar", ["/1"], [])))
+        # ... unless you allow both of them
+        self.assertTrue(self.check_scenario(("1/foo", ["/1/foo", "/1/bar"],
+                                             []),
+                                            ("1/bar", ["/1"], [])))
+        # In current python the set of following filters produces
+        # ['/1/1', '/1/1/foo', '/1'] which verifies the `/1` is skipped as
+        # higher level of filter already decided to include it.
+        self.assertTrue(self.check_scenario(("/1/1/foo", ["/1/1/foo", "/1",
+                                                          "/1/1"], [])))
+        # Three levels
+        self.assertTrue(self.check_scenario(("/1/1/foo", ["/1/1/foo"], [],
+                                             "/1/2/bar", ["/1/2/bar"], [],
+                                             "/2/baz", ["/2/baz"], [])))
+
+    def test_bad_filter(self):
+        # "bar" is missing the "/", therefor it's parent is not / but ""
+        self.assertTrue(self.check_scenario(("foo", ["bar"], []),))
+        # Filter-out "foo" won't filter-out /foo as it's not parent of /
+        self.assertTrue(self.check_scenario(("foo", [], ["foo"]),))
+        # Similar cases with double "//"
+        self.assertTrue(self.check_scenario(("foo", [], ["//foo"]),))
+        self.assertTrue(self.check_scenario(("foo", ["//foo"], []),))
+
+    def test_filter_order(self):
+        # First we evaluate filter-out and then filter-only
+        self.assertFalse(self.check_scenario(("foo", ["/foo"], ["/foo"])))
+
+
 class TestPathParent(unittest.TestCase):
 
     def test_empty_string(self):