[Piglit] [PATCH] profile.py: Support automatic naming of tests with string arguments

[Jose Fonseca]

On 06/03/15 19:36, Dylan Baker wrote:
> I made a somewhat silly assumption when writing the group_manager, that
> all Test classes would use list arguments. While this assumption is true
> for tests that require an entire command as their first argument it
> isn't true of GleanTest, where only the name of the subtest is passed.
> In this case it will be a string, and if a name isn't passed as well
> then name will end up being " ".join()'ed, ie, "api" -> "a p i".
>
> This patch adds support to the group_manager to support this feature.
>
> I considered two other approaches here. The first was to say screw
> correctness, and just add a duplicate name to the handful of tests that
> use the group_manager. The second was to change GleanTest to take a 1
> element list. Both of these had downsides that I couldn't justify.
>
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
> ---
>
> Michel, does this fix your problem?

I'm not familiar enough with the implementations details to review but I 
can confirm your patch fixes the problem I reported earlier.  Before:

$ ./piglit-print-commands.py tests/llvmpipe.py | grep '^glean/p'
warning: test glean/pointAtten does not exist
warning: test glean/texCombine does not exist
glean/p a t h s ::: bin/glean -o -v -v -v -t +paths --quick
glean/p i x e l f o r m a t s ::: bin/glean -o -v -v -v -t +pixelFormats 
--quick
glean/p o i n t s p r i t e ::: bin/glean -o -v -v -v -t +pointSprite 
--quick
glean/p o i n t a t t e n ::: bin/glean -o -v -v -v -t +pointAtten --quick

After:

$ ./piglit-print-commands.py tests/llvmpipe.py | grep '^glean/p'
glean/pixelformats ::: bin/glean -o -v -v -v -t +pixelFormats --quick
glean/paths ::: bin/glean -o -v -v -v -t +paths --quick
glean/pointsprite ::: bin/glean -o -v -v -v -t +pointSprite --quick


So

Tested-by: Jose Fonseca <jfonseca at vmware.com>

Thanks for the quick fix Dylan.

Jose

>
>   framework/profile.py             | 17 +++++++++++++----
>   framework/tests/profile_tests.py | 11 +++++++++++
>   2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/framework/profile.py b/framework/profile.py
> index e8e8ba1..e125a67 100644
> --- a/framework/profile.py
> +++ b/framework/profile.py
> @@ -322,11 +322,20 @@ class TestProfile(object):
>                         constructor as keyword args.
>
>               """
> +            # If there is no name, then either
> +            # a) join the arguments list together to make the name
> +            # b) use the argument string as the name
> +            # The former is used by the Piglit{G,C}LTest classes, the latter by
> +            # GleanTest
>               if not name:
> -                lgroup = grouptools.join(group, ' '.join(args))
> -            else:
> -                assert isinstance(name, basestring)
> -                lgroup = grouptools.join(group, name)
> +                if isinstance(args, list):
> +                    name = ' '.join(args)
> +                else:
> +                    assert isinstance(args, basestring)
> +                    name = args
> +
> +            assert isinstance(name, basestring)
> +            lgroup = grouptools.join(group, name)
>
>               self.test_list[lgroup] = test_class(
>                   args,
> diff --git a/framework/tests/profile_tests.py b/framework/tests/profile_tests.py
> index d2878c3..96337c6 100644
> --- a/framework/tests/profile_tests.py
> +++ b/framework/tests/profile_tests.py
> @@ -33,6 +33,7 @@ import framework.dmesg as dmesg
>   import framework.profile as profile
>   from framework.tests import utils
>   from framework import grouptools
> +from framework.test import GleanTest
>
>   # Don't print sys.stderr to the console
>   sys.stderr = sys.stdout
> @@ -241,3 +242,13 @@ def test_testprofile_groupmanager_kwargs_overwrite():
>
>       test = prof.test_list[grouptools.join('foo', 'a')]
>       nt.assert_equal(test.run_concurrent, False)
> +
> +
> +def test_testprofile_groupmanager_name_str():
> +    """TestProfile.group_manager: if args is a string it is not joined."""
> +    prof = profile.TestProfile()
> +    # Yes, this is really about supporting gleantest anyway.
> +    with prof.group_manager(GleanTest, 'foo') as g:
> +        g('abc')
> +
> +    nt.ok_('foo/abc' in prof.test_list)
>