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 <[email protected]>
---
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 <[email protected]>
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)
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit
