framework: Pull {include,exclude}_filter out of Options

Since these are also just special cases of filters for the standard
TestProfile filtering mechanism, and they have a lot of unique classes.
This is just a waste, the same can be achieved with a much simpler class
structure.

Signed-off-by: Dylan Baker <dylanx.c.baker@intel.com>

5 files changed, 79 insertions, 172 deletions

diff --git a/framework/options.py b/framework/options.py
index dc97c382b..db4bf7633 100644
--- a/framework/options.py
+++ b/framework/options.py
@@ -28,9 +28,7 @@ is that while you can mutate
 from __future__ import (
     absolute_import, division, print_function, unicode_literals
 )
-import collections
 import os
-import re
 
 import six
 
@@ -39,129 +37,6 @@ __all__ = ['OPTIONS']
 # pylint: disable=too-few-public-methods
 
 
-_RETYPE = type(re.compile(''))
-
-
-class _ReList(collections.MutableSequence):
-    """A list-like container that only holds RegexObjects.
-
-    This class behaves identically to a list, except that all objects are
-    forced to be RegexObjects with a flag of re.IGNORECASE (2 if one inspects
-    the object).
-
-    If inputs do not match this object, they will be coerced to becoming such
-    an object, or they assignment will fail.
-
-    """
-    def __init__(self, iterable=None):
-        self._wrapped = []
-        if iterable is not None:
-            self.extend(iterable)
-
-    @staticmethod
-    def __compile(value):
-        """Ensure that the object is properly compiled.
-
-        If the object is not a RegexObject then compile it to one, setting the
-        proper flag. If it is a RegexObject, and the flag is incorrect
-        recompile it to have the proper flags. Otherwise return it.
-
-        """
-        if not isinstance(value, _RETYPE):
-            return re.compile(value, re.IGNORECASE)
-        elif value.flags != re.IGNORECASE:
-            return re.compile(value.pattern, re.IGNORECASE)
-        return value
-
-    def __getitem__(self, index):
-        return self._wrapped[index]
-
-    def __setitem__(self, index, value):
-        self._wrapped[index] = self.__compile(value)
-
-    def __delitem__(self, index):
-        del self._wrapped[index]
-
-    def __len__(self):
-        return len(self._wrapped)
-
-    def insert(self, index, value):
-        self._wrapped.insert(index, self.__compile(value))
-
-    def __eq__(self, other):
-        """Two ReList instances are the same if their wrapped list are equal."""
-        if isinstance(other, _ReList):
-            # There doesn't seem to be a better way to do this.
-            return self._wrapped == other._wrapped  # pylint: disable=protected-access
-        raise TypeError('Cannot compare _ReList and non-_ReList object')
-
-    def __ne__(self, other):
-        return not self == other
-
-    def to_json(self):
-        """Allow easy JSON serialization.
-
-        This returns the pattern (the string or unicode used to create the re)
-        of each re object in a list rather than the RegexObject itself. This is
-        critical for JSON serialization, and thanks to the piglit_encoder this
-        is all we need to serialize this class.
-
-        """
-        return [l.pattern for l in self]
-
-
-class _FilterReList(_ReList):
-    """A version of ReList that handles group madness.
-
-    Groups are printed with '/' as a separator, but internally something else
-    may be used. This version replaces '/' with '.'.
-
-    """
-    def __setitem__(self, index, value):
-        # Replace '/' with '.', this solves the problem of '/' not matching
-        # grouptools.SEPARATOR, but without needing to import grouptools
-        super(_FilterReList, self).__setitem__(index, value.replace('/', '.'))
-
-    def insert(self, index, value):
-        super(_FilterReList, self).insert(index, value.replace('/', '.'))
-
-
-class _ReListDescriptor(object):
-    """A Descriptor than ensures reassignment of _{in,ex}clude_filter is an
-    _ReList
-
-    Without this some behavior's can get very strange. This descriptor is
-    mostly hit by testing code, but may be of use outside of testing at some
-    point.
-
-    """
-    def __init__(self, name, type_=_ReList):
-        self.__name = name
-        self.__type = type_
-
-    def __get__(self, instance, cls):
-        try:
-            return getattr(instance, self.__name)
-        except AttributeError as e:
-            new = _ReList()
-            try:
-                setattr(instance, self.__name, new)
-            except Exception:
-                raise e
-            return new
-
-    def __set__(self, instance, value):
-        assert isinstance(value, (collections.Sequence, collections.Set))
-        if isinstance(value, self.__type):
-            setattr(instance, self.__name, value)
-        else:
-            setattr(instance, self.__name, self.__type(value))
-
-    def __delete__(self, instance):
-        raise NotImplementedError('Cannot delete {} from {}'.format(
-            self.__name, instance.__class__))
-
-
 class _Options(object):  # pylint: disable=too-many-instance-attributes
     """Contains all options for a piglit run.
 
@@ -172,9 +47,6 @@ class _Options(object):  # pylint: disable=too-many-instance-attributes
 
     Options are as follows:
     execute -- False for dry run
-    include_filter -- list of compiled regex which include exclusively tests
-                      that match
-    exclude_filter -- list of compiled regex which exclude tests that match
     valgrind -- True if valgrind is to be used
     dmesg -- True if dmesg checking is desired. This forces concurrency off
     monitored -- True if monitoring is desired. This forces concurrency off
@@ -182,13 +54,8 @@ class _Options(object):  # pylint: disable=too-many-instance-attributes
     deqp_mustpass -- True to enable the use of the deqp mustpass list feature.
     """
 
-    include_filter = _ReListDescriptor('_include_filter', type_=_FilterReList)
-    exclude_filter = _ReListDescriptor('_exclude_filter', type_=_FilterReList)
-
     def __init__(self):
         self.execute = True
-        self._include_filter = _ReList()
-        self._exclude_filter = _ReList()
         self.valgrind = False
         self.dmesg = False
         self.monitored = False
@@ -216,9 +83,5 @@ class _Options(object):  # pylint: disable=too-many-instance-attributes
             if not key.startswith('_'):
                 yield key, values
 
-        # Handle the attributes that have a descriptor separately
-        yield 'include_filter', self.include_filter
-        yield 'exclude_filter', self.exclude_filter
-
 
 OPTIONS = _Options()
diff --git a/framework/profile.py b/framework/profile.py
index e00fbc4d7..17d17db37 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -37,21 +37,59 @@ import itertools
 import multiprocessing
 import multiprocessing.dummy
 import os
+import re
 
 import six
 
-from framework import grouptools, exceptions, options
+from framework import grouptools, exceptions
 from framework.dmesg import get_dmesg
 from framework.log import LogManager
 from framework.monitoring import Monitoring
 from framework.test.base import Test
 
 __all__ = [
+    'RegexFilter',
     'TestProfile',
     'load_test_profile',
 ]
 
 
+class RegexFilter(object):
+    """An object to be passed to TestProfile.filter.
+
+    This object takes a list (or list-like object) of strings which it converts
+    to re.compiled objects (so use raw strings for escape sequences), and acts
+    as a callable for filtering tests. If a test matches any of the regex then
+    it will be scheduled to run. When the inverse keyword argument is True then
+    a test that matches any regex will not be scheduled. Regardless of the
+    value of the inverse flag if filters is empty then the test will be run.
+
+    Arguments:
+    filters -- a list of regex compiled objects.
+
+    Keyword Arguments:
+    inverse -- Inverse the sense of the match.
+    """
+
+    def __init__(self, filters, inverse=False):
+        self.filters = [re.compile(f) for f in filters]
+        self.inverse = inverse
+
+    def __call__(self, name, _):  # pylint: disable=invalid-name
+        # This needs to match the signature (name, test), since it doesn't need
+        # the test instance use _.
+
+        # If self.filters is empty then return True, we don't want to remove
+        # any tests from the run.
+        if not self.filters:
+            return True
+
+        if not self.inverse:
+            return any(r.search(name) for r in self.filters)
+        else:
+            return not any(r.search(name) for r in self.filters)
+
+
 class TestDict(collections.MutableMapping):
     """A special kind of dict for tests.
 
@@ -252,22 +290,10 @@ class TestProfile(object):
         runs it's own filters plus the filters in the self.filters name
 
         """
-        def matches_any_regexp(x, re_list):
-            return any(r.search(x) for r in re_list)
-
-        # The extra argument is needed to match check_all's API
-        def test_matches(path, test):
-            """Filter for user-specified restrictions"""
-            return ((not options.OPTIONS.include_filter or
-                     matches_any_regexp(path, options.OPTIONS.include_filter))
-                    and not matches_any_regexp(path, options.OPTIONS.exclude_filter))
-
-        filters = self.filters + [test_matches]
-
         def check_all(item):
             """ Checks group and test name against all filters """
             path, test = item
-            for f in filters:
+            for f in self.filters:
                 if not f(path, test):
                     return False
             return True
diff --git a/framework/programs/print_commands.py b/framework/programs/print_commands.py
index 6e68eb56d..5811cd2b1 100644
--- a/framework/programs/print_commands.py
+++ b/framework/programs/print_commands.py
@@ -86,15 +86,17 @@ def main(input_):
                         help="Path to results folder")
     args = parser.parse_args(input_)
 
-    options.OPTIONS.exclude_filter = args.exclude_tests
-    options.OPTIONS.include_filter = args.include_tests
+    profile_ = profile.load_test_profile(args.testProfile)
+
+    if args.exclude_tests:
+        profile_.filters.append(profile.RegexFilter(args.exclude_tests))
+    if args.include_tests:
+        profile_.filters.append(profile.RegexFilter(args.include_tests))
 
     # Change to the piglit's path
     piglit_dir = os.path.dirname(os.path.realpath(sys.argv[0]))
     os.chdir(piglit_dir)
 
-    profile_ = profile.load_test_profile(args.testProfile)
-
     profile_.prepare_test_list()
     for name, test in six.iteritems(profile_.test_list):
         assert isinstance(test, Test)
diff --git a/framework/programs/run.py b/framework/programs/run.py
index 20a036e2a..2ef3b4ecd 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -23,12 +23,13 @@ from __future__ import (
     absolute_import, division, print_function, unicode_literals
 )
 import argparse
-import sys
+import ctypes
 import os
 import os.path as path
-import time
-import ctypes
+import re
 import shutil
+import sys
+import time
 
 import six
 
@@ -223,6 +224,8 @@ def _create_metadata(args, name):
     opts['profile'] = args.test_profile
     opts['log_level'] = args.log_level
     opts['concurrent'] = args.concurrency
+    opts['include_filter'] = args.include_tests
+    opts['exclude_filter'] = args.exclude_tests
     if args.platform:
         opts['platform'] = args.platform
 
@@ -277,8 +280,6 @@ def run(input_):
         args.concurrency = "none"
 
     # Pass arguments into Options
-    options.OPTIONS.exclude_filter = args.exclude_tests
-    options.OPTIONS.include_filter = args.include_tests
     options.OPTIONS.execute = args.execute
     options.OPTIONS.valgrind = args.valgrind
     options.OPTIONS.dmesg = args.dmesg
@@ -335,6 +336,13 @@ def run(input_):
         for p in profiles:
             p.monitoring = args.monitored
 
+    for p in profiles:
+        if args.exclude_tests:
+            p.filters.append(profile.RegexFilter(args.exclude_tests,
+                                                 inverse=True))
+        if args.include_tests:
+            p.filters.append(profile.RegexFilter(args.include_tests))
+
     time_elapsed = TimeAttribute(start=time.time())
 
     profile.run(profiles, args.log_level, backend, args.concurrency)
@@ -366,8 +374,6 @@ def resume(input_):
     _disable_windows_exception_messages()
 
     results = backends.load(args.results_path)
-    options.OPTIONS.exclude_filter = results.options['exclude_filter']
-    options.OPTIONS.include_filter = results.options['include_filter']
     options.OPTIONS.execute = results.options['execute']
     options.OPTIONS.valgrind = results.options['valgrind']
     options.OPTIONS.dmesg = results.options['dmesg']
@@ -407,7 +413,15 @@ def resume(input_):
         if options.OPTIONS.monitored:
             p.monitoring = options.OPTIONS.monitored
 
-        p.filters.append(lambda n, _: n not in exclude_tests)
+        if exclude_tests:
+            p.filters.append(lambda n, _: n not in exclude_tests)
+        if results.options['exclude_filter']:
+            p.filters.append(
+                profile.RegexFilter(results.options['exclude_filter'],
+                                    inverse=True))
+        if results.options['include_filter']:
+            p.filters.append(
+                profile.RegexFilter(results.options['include_filter']))
 
     # This is resumed, don't bother with time since it won't be accurate anyway
     profile.run(
diff --git a/framework/summary/feature.py b/framework/summary/feature.py
index a66a49b2f..7d426e73e 100644
--- a/framework/summary/feature.py
+++ b/framework/summary/feature.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2015-2016 Intel Corporation
+# Copyright (c) 2016-2016 Intel Corporation
 
 # Permission is hereby granted, free of charge, to any person
 # obtaining a copy of this software and associated documentation
@@ -30,7 +30,7 @@ try:
 except ImportError:
     import json
 
-from framework import options, exceptions, profile, status
+from framework import exceptions, profile, status
 
 
 class FeatResults(object):  # pylint: disable=too-few-public-methods
@@ -57,16 +57,18 @@ class FeatResults(object):  # pylint: disable=too-few-public-methods
         for feature in feature_data:
             self.features.add(feature)
 
+            profiles[feature] = profile_orig.copy()
+
             incl_str = feature_data[feature]["include_tests"]
             excl_str = feature_data[feature]["exclude_tests"]
 
-            include_filter = [incl_str] if incl_str and not incl_str.isspace() else []
-            exclude_filter = [excl_str] if excl_str and not excl_str.isspace() else []
-
-            options.OPTIONS.exclude_filter = exclude_filter
-            options.OPTIONS.include_filter = include_filter
-
-            profiles[feature] = profile_orig.copy()
+            profiles[feature].filters.append(
+                profile.RegexFilter(
+                    [incl_str] if incl_str and not incl_str.isspace() else []))
+            profiles[feature].filters.append(
+                profile.RegexFilter(
+                    [excl_str] if excl_str and not excl_str.isspace() else [],
+                    inverse=True))
 
             # An empty list will raise PiglitFatalError exception
             # But for reporting we need to handle this situation