[Piglit] [PATCH 1/4] framework: Allow ExecTest commands to be space-separated strings.

[Paul Berry]

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

> ---
>  framework/exectest.py |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/framework/exectest.py b/framework/exectest.py
> index dc9dc9e..59cb090 100644
> --- a/framework/exectest.py
> +++ b/framework/exectest.py
> @@ -23,6 +23,8 @@
>
>  import os
>  import subprocess
> +import string
> +import types
>
>  from core import Test, testBinDir, TestResult
>
> @@ -36,6 +38,9 @@ class ExecTest(Test):
>                self.command = command
>                self.env = {}
>
> +               if type(self.command) is types.StringType:
> +                       self.command = string.split(self.command)
> +
>

The standard Python idiom for testing whether a thing is a string is
"isinstance(..., basestring)" rather than "type(...) is types.StringType".
This would ensure that ExecTest() would work properly even if passed a
unicode string.  (Unicode strings crop up in Python more often than you
might realize, since any string manipulation that involves both unicode and
non-unicode strings produces a unicode string as a result, even if the
result doesn't contain any non-ascii characters).

Also, string.split(s) is deprecated in favor of s.split(), and will be
removed in Python 3.0 (see
http://docs.python.org/library/string.html#deprecated-string-functions).



Just to go on record with what I said in person on Thursday, I think it
would be preferable to use shlex.split() rather than string.split(), so that
you get standard shell-style string splitting behavior, for example:

    shlex.split('foo "bar baz"') == ['foo', 'bar baz']

This wouldn't break any of the uses of ExecTest() that you are enabling, and
it would make life easier in the future for anyone who needed to pass a
multi-word argument to a test program using ExecTest().
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20110912/51f5d9e5/attachment.html>