On Sat, Jan 18, 2014 at 5:03 AM, Dylan Baker <baker.dyla...@gmail.com> wrote: > On Saturday, January 18, 2014 04:31:37 AM Ilia Mirkin wrote: >> The sole user is tests/gpu.py, which was passing in a predicate that >> worked (a) as a filter, and (b) expected to receive a test. However the >> implementation operated on the dict-of-dicts. Instead keep track of all >> the filtering functions, and apply the filters when flattening the tests >> tree. >> >> Take this opportunity to pass in a second argument to the filter >> function: the full test path. This should allow future conversion of the >> 'del' style of filtering. >> >> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu> >> --- >> >> Dylan, >> >> I believe I've integrated all your suggestions with the exception of getting >> rid of the argument name from the filter_tests lambda. I think it's better >> to have it be there and be named. I suspect it's the sort of thing that >> will be copied around, and then potentially flipped to want the name of the >> test. Would be better not to have to think about arg names. If people are >> _really_ confused, looking at the definition of filter_tests should clear >> that right up. >> >> framework/core.py | 32 ++++++++++++++++++++------------ >> tests/gpu.py | 2 +- >> 2 files changed, 21 insertions(+), 13 deletions(-) >> >> diff --git a/framework/core.py b/framework/core.py >> index 368c996..0c71ea6 100644 >> --- a/framework/core.py >> +++ b/framework/core.py >> @@ -519,6 +519,7 @@ class TestProfile: >> def __init__(self): >> self.tests = Group() >> self.test_list = {} >> + self.filters = [] >> >> def flatten_group_hierarchy(self): >> ''' >> @@ -548,14 +549,23 @@ class TestProfile: >> def matches_any_regexp(x, re_list): >> return True in map(lambda r: r.search(x) is not None, re_list) >> >> - def test_matches(item): >> - path, test = item >> + def test_matches(path, test): >> + """Filter for user-specified restrictions""" >> return ((not env.filter or matches_any_regexp(path, >> env.filter)) and not path in env.exclude_tests and >> not matches_any_regexp(path, env.exclude_filter)) >> >> + filters = self.filters + [test_matches] >> + def check_all(item): >> + path, test = item >> + for f in filters: >> + if not f(path, test): >> + return False >> + return True >> + >> # Filter out unwanted tests >> - self.test_list = dict(filter(test_matches, self.test_list.items())) >> + self.test_list = dict(item for item in self.test_list.iteritems() >> + if check_all(item)) >> >> def run(self, env, json_writer): >> ''' >> @@ -623,17 +633,15 @@ class TestProfile: >> group = group[group_name] >> del group[l[-1]] >> >> - def remove_tests(self, function): >> - """ Remove tests that return true from function >> - >> - This is a destructive method, and passing an incorrect function >> could - result in undesired behavior. >> + def filter_tests(self, function): >> + """Filter out tests that return false from the supplied function >> >> + Arguments: >> + function -- a callable that takes two parameters: path, test and >> + returns whether the test should be included in the test >> + run or not. >> """ >> - # What we really want is a dictionary comprehension, but since >> python - # 2.6 is officially supported we can't use one >> - # {k:v for k, v in self.tests.iteritems() if function(v)} >> - self.tests = dict((k,v) for k, v in self.tests.iteritems() if >> function(v)) + self.filters.append(function) >> >> def update(self, *profiles): >> """ Updates the contents of this TestProfile instance with another >> diff --git a/tests/gpu.py b/tests/gpu.py >> index d7690ae..55ead29 100644 >> --- a/tests/gpu.py >> +++ b/tests/gpu.py >> @@ -8,7 +8,7 @@ from framework.glsl_parser_test import GLSLParserTest >> __all__ = ['profile'] >> >> # Remove all glsl_parser_tests, as they are compiler test >> -profile.remove_tests(lambda x: not isinstance(x, GLSLParserTest)) >> +profile.filter_tests(lambda p, t: not isinstance(t, GLSLParserTest)) >> >> # Drop ARB_vertex_program/ARB_fragment_program compiler tests. >> del profile.tests['spec']['ARB_vertex_program'] > > This looks good. > > Reviewed-by: Dylan Baker <baker.dyla...@gmail.com>
Thanks! Would you be able to check this in? I don't have an account (yet). _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit