[Piglit] [PATCH 2/4] all.tests: Make concurrent test setup a general helper function.

[Paul Berry]

On 8 September 2011 23:07, Eric Anholt <eric at anholt.net> wrote:

> ---
>  tests/all.tests |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tests/all.tests b/tests/all.tests
> index 03dcd59..fc66ff6 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -25,6 +25,12 @@ generatedTestDir = os.path.join(
>                os.path.join(os.path.dirname(__file__), '..')),
>        'generated_tests')
>
> +# Quick wrapper for PlainExecTest for our usual concurrent args.
> +def concurrent_test(args):
> +       test = PlainExecTest(args + " -auto -fbo")
> +       test.runConcurrent = True
> +       return test
> +
>  ######
>  # Collecting all tests
>  profile = TestProfile()
> @@ -293,12 +299,6 @@ add_fbo_depthstencil_tests(general, 'default_fb')
>
>  shaders = Group()
>
> -def add_shader_test(group, filepath, test_name):
> -       """Add a shader test to the given group."""
> -       group[test_name] = PlainExecTest(['shader_runner', '-auto', '-fbo',
> -                                         filepath])
> -       group[test_name].runConcurrent = True
> -
>  def add_shader_test_dir(group, dirpath, recursive=False):
>        """Add all shader tests in a directory to the given group."""
>        for filename in os.listdir(dirpath):
> @@ -314,7 +314,7 @@ def add_shader_test_dir(group, dirpath,
> recursive=False):
>                        if ext != 'shader_test':
>                                continue
>                        testname = filename[0:-(len(ext) + 1)] # +1 for '.'
> -                       add_shader_test(group, filepath, testname)
> +                       group[testname] = concurrent_test('shader_runner '
> + filepath)
>
>  def add_getactiveuniform_count(group, name, expected):
>        path = 'shaders/'
> --
> 1.7.5.4
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>

This patch introduces a limitation to add_shader_test_dir(): it no longer
works properly when a test name or directory contains spaces.  Although
spaces in file/directory names are generally frowned upon in the UNIX world
(and Piglit currently doesn't contain any such files or directories), I
don't think that's any excuse to make the code fragile.

In addition, the new function concurrent_test() has the limitation that it
can *only* be called with a string, not a list of strings.  Since the
intended purpose of concurrent_test() is to take the place of ExecTest() for
tests that should be executed concurrently, it would avoid confusion if it
accepted inputs in the same form that ExecTest() accepts.

In my experience, the best way to avoid issues like these is to convert from
a space-separated string to a list as early as possible (at the top of
concurrent_test()), and then manipulate everything as lists of strings
thereafter.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20110912/f7891b5f/attachment.htm>