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.ba...@intel.com> --- framework/options.py | 137 +---------------------- framework/profile.py | 54 +++++--- framework/programs/print_commands.py | 10 +- framework/programs/run.py | 30 +++-- framework/summary/feature.py | 20 +-- unittests/framework/test_options.py | 178 +---------------------------- unittests/framework/test_profile.py | 126 ++++++-------------- 7 files changed, 119 insertions(+), 436 deletions(-) diff --git a/framework/options.py b/framework/options.py index dc97c38..db4bf76 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 e00fbc4..17d17db 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 6e68eb5..5811cd2 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 20a036e..2ef3b4e 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 a66a49b..7d426e7 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 diff --git a/unittests/framework/test_options.py b/unittests/framework/test_options.py index 65ff946..bf296c1 100644 --- a/unittests/framework/test_options.py +++ b/unittests/framework/test_options.py @@ -23,9 +23,6 @@ from __future__ import ( absolute_import, division, print_function, unicode_literals ) -import re - -import pytest from framework import options @@ -33,180 +30,6 @@ from framework import options # pylint: disable=invalid-name # pylint: disable=no-self-use -_RETYPE = type(re.compile('')) - - -def test_ReList_iterable_argument(): - """options._ReList: handles an iterable argument correctly""" - test = options._ReList(['foo']) - assert isinstance(test[0], _RETYPE) - - -class TestReList(object): - """Tests for the ReList class. - - These particular tests don't mutate the state of ReList, and thus can be - run with the same instance over and over, other tests that do mutate the - state need a per test ReList instance. - - """ - @classmethod - def setup_class(cls): - cls.test = options._ReList(['foo']) - - def test_eq(self): - """Test options._ReList.__eq__.""" - test1 = ['foo'] - test2 = options._ReList(['foo']) - - with pytest.raises(TypeError): - assert self.test == test1 - - assert self.test == test2 - - def test_ne(self): - """Test hoptions._ReList.__ne__.""" - test1 = ['bar'] - test2 = options._ReList(['bar']) - - with pytest.raises(TypeError): - assert self.test != test1 - - assert self.test != test2 - - def test_getitem(self): - """options._ReList.__getitem__: returns expected value.""" - assert isinstance(self.test[0], _RETYPE) - - def test_flags(self): - """options._ReList.__getitem__: sets flags correctly.""" - assert self.test[0].flags & re.IGNORECASE != 0 - - def test_len(self): - """options._ReList.len: returns expected values.""" - assert len(self.test) == 1 - - def test_to_json(self): - """options._ReList.to_json: returns expected values.""" - assert self.test.to_json() == ['foo'] - - -class TestReListMutate(object): - """Tests for ReList that mutate state.""" - test = None - - def setup(self): - self.test = options._ReList(['foo']) - - def test_relist_insert(self): - """options._ReList.len: inserts value as expected""" - obj = re.compile('bar', re.IGNORECASE) - - self.test.insert(0, obj) - - assert self.test[0] == obj - - def test_relist_delitem(self): - """options._ReList.len: removes value as expected""" - del self.test[0] - - assert len(self.test) == 0 - - def test_relist_setitem(self): - """options._ReList.__setitem__: replaces values""" - sentinel = re.compile('bar') - self.test[0] = sentinel - - # The pattern must be tested because the flags on the re object might - # require it to be recompiled, thus they might not be the same object, - # or even be equal according to python (though they are for the - # purposes of this test) - assert self.test[0].pattern == sentinel.pattern - - -class TestReListDescriptor(object): - """Test the ReListDescriptor class. - - Since this class is a descriptor it needs to be attached to an object at - the class level. - - """ - test = None - - @classmethod - def setup_class(cls): - """Create a test object.""" - class _Test(object): - desc = options._ReListDescriptor('test_desc') - notexists = options._ReListDescriptor('test_notexists') - - def __init__(self): - self.test_desc = options._ReList() - - cls._test = _Test - - def setup(self): - self.test = self._test() - - def test_get_exists(self): - """options._ReListDescriptor.__get__: Returns value if it exists.""" - assert self.test.desc == self.test.test_desc - - def test_get_not_exists(self): - """options._ReListDescriptor.__get__: Returns new _ReList if it doesn't - exists.""" - assert self.test.notexists == self.test.test_notexists # pylint: disable=no-member - - def test_get_not_exists_fail(self, mocker): - """options._ReListDescriptor.__get__: Raises AttributError if name - doesn't exist and can't be created.""" - mocker.patch('framework.options.setattr', - mocker.Mock(side_effect=Exception), - create=True) - - with pytest.raises(AttributeError): - self.test.notexists # pylint: disable=pointless-statement - - def test_set_relist(self): - """options._ReListDescriptor.__set__: assigns an ReList without - copying.""" - val = options._ReList(['foo']) - self.test.desc = val - assert self.test.desc is val - - def test_set_other(self): - """options._ReListDescriptor.__set__: converts other types to ReList""" - val = options._ReList(['foo']) - self.test.desc = ['foo'] - assert self.test.desc == val - - def test_delete(self): - """options._ReListDescriptor.__delete___: raises NotImplementedError""" - with pytest.raises(NotImplementedError): - del self.test.desc - - -class TestFilterReList(object): - """Tests for FilterReList. - - provides a unique instance per test, which protects against state mutation. - - """ - test = None - - def setup(self): - self.test = options._FilterReList(['foo']) - - def test_setitem(self): - """options._FilterReList.__setitem__: replaces '/' with '.'.""" - self.test[0] = 'foo/bar' - assert self.test[0].pattern == 'foo.bar' - - def test_filterrelist_insert(self): - """options._FilterReList.insert: replaces '/' with '.'.""" - self.test.insert(0, 'foo/bar') - assert self.test[0].pattern == 'foo.bar' - def test_options_clear(): """options.Options.clear(): resests options values to init state.""" @@ -215,7 +38,6 @@ def test_options_clear(): test = options._Options() test.execute = False test.sync = True - test.exclude_filter.append('foo') test.clear() assert list(iter(baseline)) == list(iter(test)) diff --git a/unittests/framework/test_profile.py b/unittests/framework/test_profile.py index f2aa5b5..ea4ee70 100644 --- a/unittests/framework/test_profile.py +++ b/unittests/framework/test_profile.py @@ -23,11 +23,6 @@ from __future__ import ( absolute_import, division, print_function, unicode_literals ) -import copy -try: - from unittest import mock -except ImportError: - import mock import pytest import six @@ -35,7 +30,6 @@ import six from framework import dmesg from framework import exceptions from framework import grouptools -from framework import options from framework import profile from framework.test.gleantest import GleanTest from . import utils @@ -101,86 +95,6 @@ class TestTestProfile(object): profile_.dmesg = False assert isinstance(profile_.dmesg, dmesg.DummyDmesg) - class TestPrepareTestList(object): - """Create tests for TestProfile.prepare_test_list filtering.""" - - @classmethod - def setup_class(cls): - cls.opts = None - cls.data = None - cls.__patcher = mock.patch('framework.profile.options.OPTIONS', - new_callable=options._Options) - - def setup(self): - """Setup each test.""" - self.data = profile.TestDict() - self.data[grouptools.join('group1', 'test1')] = \ - utils.Test(['thingy']) - self.data[grouptools.join('group1', 'group3', 'test2')] = \ - utils.Test(['thing']) - self.data[grouptools.join('group3', 'test5')] = \ - utils.Test(['other']) - self.data[grouptools.join('group4', 'Test9')] = \ - utils.Test(['is_caps']) - self.opts = self.__patcher.start() - - def teardown(self): - self.__patcher.stop() - - def test_matches_filter_mar_1(self): - """profile.TestProfile.prepare_test_list: 'not env.filter or - matches_any_regex()' env.filter is False. - - Nothing should be filtered. - """ - profile_ = profile.TestProfile() - profile_.test_list = self.data - profile_.prepare_test_list() - - assert dict(profile_.test_list) == dict(self.data) - - def test_matches_filter_mar_2(self): - """profile.TestProfile.prepare_test_list: 'not env.filter or - matches_any_regex()' mar is False. - """ - self.opts.include_filter = ['test5'] - - profile_ = profile.TestProfile() - profile_.test_list = self.data - profile_.prepare_test_list() - - baseline = { - grouptools.join('group3', 'test5'): utils.Test(['other'])} - - assert dict(profile_.test_list) == baseline - - def test_matches_exclude_mar(self): - """profile.TestProfile.prepare_test_list: 'not - matches_any_regexp()'. - """ - self.opts.exclude_filter = ['test5'] - - baseline = copy.deepcopy(self.data) - del baseline[grouptools.join('group3', 'test5')] - - profile_ = profile.TestProfile() - profile_.test_list = self.data - profile_.prepare_test_list() - - assert dict(profile_.test_list) == dict(baseline) - - def test_matches_include_caps(self): - """profile.TestProfile.prepare_test_list: matches capitalized - tests. - """ - self.opts.exclude_filter = ['test9'] - - profile_ = profile.TestProfile() - profile_.test_list = self.data - profile_.prepare_test_list() - - assert grouptools.join('group4', 'Test9') not in profile_.test_list - class TestGroupManager(object): """Tests for TestProfile.group_manager.""" @@ -386,3 +300,43 @@ class TestTestDict(object): test['a'] = utils.Test(['bar']) assert test['a'].command == ['bar'] + + +class TestRegexFilter(object): + """Tests for the RegexFilter class.""" + + class TestNormal(object): + """Tests for inverse set to False (default).""" + + def test_empty(self): + """Returns True when no filters are provided.""" + test = profile.RegexFilter([]) + assert test('foobob', None) + + def test_matches(self): + """Returns True when the test matches any regex.""" + test = profile.RegexFilter([r'foo', r'bar']) + assert test('foobob', None) + + def test_not_matches(self): + """Returns True when the test matches any regex.""" + test = profile.RegexFilter([r'fob', r'bar']) + assert not test('foobob', None) + + class TestInverse(object): + """Tests for inverse set to True.""" + + def test_empty(self): + """Returns True when no filters are provided.""" + test = profile.RegexFilter([], inverse=True) + assert test('foobob', None) + + def test_matches(self): + """Returns False when the test matches any regex.""" + test = profile.RegexFilter([r'foo', r'bar'], inverse=True) + assert not test('foobob', None) + + def test_not_matches(self): + """Returns False when the test matches any regex.""" + test = profile.RegexFilter([r'fob', r'bar'], inverse=True) + assert test('foobob', None) -- git-series 0.8.10 _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit