[Piglit] [PATCH 09/12] tests/gpu.py: Don't use execfile

[Kenneth Graunke]

On 12/23/2013 04:51 PM, Dylan Baker wrote:
> This uses import rather than exefile. It requires a few additional
> changes since gpu.py uses some hacky behavior to remove shader_tests
> 
> XXX: This isn't ready to be pushed

Isn't it?

> ---
>  framework/core.py |  9 +++++++++
>  tests/gpu.py      | 20 ++++++--------------
>  2 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/framework/core.py b/framework/core.py
> index 28a1ded..cd5c11d 100644
> --- a/framework/core.py
> +++ b/framework/core.py
> @@ -605,6 +605,15 @@ class TestProfile:
>              group = group[group_name]
>          del group[l[-1]]
>  
> +    def filter(self, function):

I don't like the name of this function.  'filter' is a well understood
standard library function in Python/Haskell/Scheme, which takes a list
and produces a new list without some elements.  The key part is: filter
is non-destructive.  This function /is/ destructive.

I'd probably use 'remove'/'delete' instead, like 'remove_by_predicate'
or some such.  Or just fold it in to the one use...

> +        """ Remove tests that return true from function
> +
> +        This is a destructive method, and passing an incorrect function could
> +        result in undesired behavior.
> +
> +        """
> +        self.tests = {k:v for k, v in self.tests.iteritems() if function(v)}

I believe this dictionary syntax is new with Python 2.7, and won't work
on 2.6.  If you want to be compatible, this ought to work:

self.tests = dict((k,v) for k, v in self.tests.iteritems() if function(v))

or

self.tests = dict(p for p in self.tests.iteritems() if function(p[1]))

> +
>      def update(self, *profiles):
>          """ Updates the contents of this TestProfile instance with another
>  
> diff --git a/tests/gpu.py b/tests/gpu.py
> index 99e1614..18a4b66 100644
> --- a/tests/gpu.py
> +++ b/tests/gpu.py
> @@ -2,23 +2,15 @@
>  
>  # quick.tests minus compiler tests.
>  
> -import framework.glsl_parser_test
> -import os.path
> +from tests.all import profile
> +from framework.glsl_parser_test import GLSLParserTest
>  
>  __all__ = ['profile']
>  
> -global profile
> -
> -# Filter out any GLSLParserTest instances, as they're compiler tests.
> -def add_glsl_parser_test(group, filepath, test_name):
> -    # Dummy version of framework.glsl_parser_test.add_glsl_parser_test
> -    pass
> -
> -# This be done before executing the base test list script
> -framework.glsl_parser_test.add_glsl_parser_test = add_glsl_parser_test
> -
> -
> -execfile(os.path.dirname(__file__) + '/quick.py')
> +# Remove all glsl_parser_tests

Please keep my comment explaining why this profile removes these tests.

> +profile.filter(lambda x: not isinstance(x, GLSLParserTest))
>  
>  # Drop ARB_vertex_program/ARB_fragment_program compiler tests.
> +del profile.tests['spec']['ARB_vertex_program']
> +del profile.tests['spec']['ARB_fragment_program']
>  del profile.tests['asmparsertest']

Other than those nits, this series looks pretty reasonable.  With those
minor things fixed, the series is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>