Another way to look at it is: Here are all the tests. Here is the profile's filter function (of which there can be many). Here is the user's filter function. test_matches is the user's filter function -- and like I said, I'd be happy to just stick that on as a predicate to the filters list, and then just have a Iterables.all() style mega-predicate function that checks all the predicates in the list before queuing the test.
Alternatively, what do you propose? -ilia On Thu, Jan 16, 2014 at 7:02 PM, Dylan Baker <[email protected]> wrote: > I'd like to maintain clear separation between the profile filtering the list, > and the user filtering the list. Currently that happens because remove_test is > called in gpu.py, and prepare_test_list() is called by piglit-<run|resume>.py. > > A profile, say gpu.py, says: "I'm quick.py, except, I don't have and > glsl_parser tests, asmparsertest, ARB_vertex_program tests or > ARB_fragment_program tests". When gpu.tests returns a profile with it's test > list it should already have removed all of those tests from quick.py's profile > instance, they don't belong to gpu.py. > > test_matches on the other hand is for the user to say, "I'm working on > GL_hamsandwhich, and I only want to run those tests", or "I don't care about > glean tests, don't run those" > > On Thursday, January 16, 2014 06:42:35 PM Ilia Mirkin wrote: >> Or perhaps I didn't understand your objection... what do you mean by >> 'same time'? It all happens before the tests run... are you saying you >> prefer having multiple intermediate copies of the dictionaries >> created? >> >> On Thu, Jan 16, 2014 at 6:37 PM, Ilia Mirkin <[email protected]> wrote: >> > 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
