[Piglit] [PATCH v3] core: fix remove_tests logic

[Dylan Baker]

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 <imirkin at 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.dylan.c at gmail.com>
-------------- 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/20140118/70c885d0/attachment-0001.pgp>