[Piglit] [PATCH] core: fix remove_tests logic

[Dylan Baker]

I'll comment here because we've generated enough quote chatter in the other 
thread.

I have a couple of minor requests and then you have my r-b:
Reviewed-by: Dylan Baker <baker.dylan.c at gmail.com>

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 <imirkin at alum.mit.edu>
> ---
> 
> 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

Will you put a couple of comments here, explaining that one is for user 
filtering and the other for profile filtering?

>              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
>          """

It's just a nit, but I really dislike the hanging """ on a single line 
docstring

> -        # 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']
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140116/d21ac168/attachment.pgp>