How would you feel about a patch that instead adds test_matches to filters and then does a filter on some function that only looks through self.filters?
On Thu, Jan 16, 2014 at 6:35 PM, Dylan Baker <[email protected]> wrote: > I'm not really thrilled with this patch, here's my reasoning: > > I think it's important to separate the removal of tests by the profile and > those of an end-user. If they use the same code path to achieve that, great! > code duplication sucks; but I don't like the idea of filtering tests for the > profile and the user at the same time. > > On Thursday, January 16, 2014 10:13:41 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. >> >> Signed-off-by: Ilia Mirkin <[email protected]> >> --- >> >> You can see the difference by running in a dry run mode with this patch and >> without on gpu.py. [Also the comment on the function was wrong... true keeps >> the test.] >> >> framework/core.py | 17 +++++++---------- >> tests/gpu.py | 2 +- >> 2 files changed, 8 insertions(+), 11 deletions(-) >> >> diff --git a/framework/core.py b/framework/core.py >> index 368c996..f3cc270 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): >> ''' >> @@ -550,6 +551,9 @@ class TestProfile: >> >> def test_matches(item): >> path, test = item >> + for f in self.filters: >> + if not f(test): >> + return False >> 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)) >> @@ -623,17 +627,10 @@ 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): >> + """ Remove tests that return false from function >> """ >> - # 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..5190e20 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 x: not isinstance(x, GLSLParserTest)) >> >> # Drop ARB_vertex_program/ARB_fragment_program compiler tests. >> del profile.tests['spec']['ARB_vertex_program'] _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
