[Piglit] [PATCH 5/7] grouptools.py: make group separator a constant value
Dylan Baker
baker.dylan.c at gmail.com
Thu Mar 12 15:42:08 PDT 2015
This patch makes two changes to the way grouptools works, one as a
result of the other. The primary change is that this uses a constant
SEPARATOR to set the value used by the various functions to work with
groups.
The second change is that this no longer wraps posixpath, which is
necessary since posixpath doesn't allow the constant to be changed.
(well, it does, but that's very fragile and not worth the pain.)
This also changes some of the tests to use the separator in their names
using the DocFormatter decorator. This will allow changing the separator
for groups with one trivial change (and the necessary JSON version
updates of course)
Also converts the junit backend to use grouptools.SEPERATOR instead of a
hardcoded value, and adds a testcase for this potential problem.
Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
---
framework/backends/junit.py | 4 +-
framework/grouptools.py | 82 ++++++++++++++++++++++++-------------
framework/tests/backends_tests.py | 26 ++++++++++++
framework/tests/grouptools_tests.py | 23 +++++++++--
framework/tests/utils.py | 38 +++++++++++++++++
5 files changed, 139 insertions(+), 34 deletions(-)
diff --git a/framework/backends/junit.py b/framework/backends/junit.py
index 53b6086..5459e80 100644
--- a/framework/backends/junit.py
+++ b/framework/backends/junit.py
@@ -22,7 +22,6 @@
from __future__ import print_function, absolute_import
import os.path
-import re
import shutil
try:
@@ -46,7 +45,6 @@ class JUnitBackend(FileBackend):
https://svn.jenkins-ci.org/trunk/hudson/dtkit/dtkit-format/dtkit-junit-model/src/main/resources/com/thalesgroup/dtkit/junit/model/xsd/junit-7.xsd
"""
- _REPLACE = re.compile(r'[/\\]')
def __init__(self, dest, junit_suffix='', **options):
super(JUnitBackend, self).__init__(dest, **options)
@@ -159,7 +157,7 @@ class JUnitBackend(FileBackend):
# replacing any '.' with '_' (so we don't get false groups).
classname, testname = grouptools.splitname(name)
classname = classname.replace('.', '_')
- classname = JUnitBackend._REPLACE.sub('.', classname)
+ classname = classname.replace(grouptools.SEPERATOR, '.')
# Add the test to the piglit group rather than directly to the root
# group, this allows piglit junit to be used in conjunction with other
diff --git a/framework/grouptools.py b/framework/grouptools.py
index cc0b4e9..6224b99 100644
--- a/framework/grouptools.py
+++ b/framework/grouptools.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2014 Intel Corporation
+# Copyright (c) 2014, 2015 Intel Corporation
# Permission is hereby granted, free of charge, to any person obtaining a copy
# of this software and associated documentation files (the "Software"), to deal
@@ -29,19 +29,20 @@ posix paths they may not start with a leading '/'.
"""
-import posixpath
import os.path
__all__ = [
- 'join',
'commonprefix',
- 'split',
+ 'from_path',
'groupname',
- 'testname',
+ 'join',
+ 'split',
'splitname',
- 'from_path'
+ 'testname',
]
+SEPERATOR = '/'
+
def _normalize(group):
"""Helper to normalize group paths on Windows.
@@ -60,17 +61,16 @@ def _normalize(group):
def _assert_illegal(group):
"""Helper that checks for illegal characters in input."""
- assert isinstance(group, (str, unicode)), 'Type must be string or unicode'
+ assert isinstance(group, (str, unicode)), \
+ 'Type must be string or unicode but was {}'.format(type(group))
assert '\\' not in group, \
'Groups are not paths and cannot contain \\. ({})'.format(group)
- assert not group.startswith('/'), \
- 'Groups cannot start with /. ({})' .format(group)
def testname(group):
"""Return the last element of a group name.
- Provided the value 'group1/group2/test1' will provide 'test1', this
+ Provided the value 'group1|group2|test1' will provide 'test1', this
does not enforce any rules that the final element is a test name, and can
be used to shaved down groups.
@@ -80,7 +80,7 @@ def testname(group):
group = _normalize(group)
_assert_illegal(group)
- return posixpath.basename(group)
+ return splitname(group)[1]
def groupname(group):
@@ -96,7 +96,7 @@ def groupname(group):
group = _normalize(group)
_assert_illegal(group)
- return posixpath.dirname(group)
+ return splitname(group)[0]
def splitname(group):
@@ -104,7 +104,11 @@ def splitname(group):
group = _normalize(group)
_assert_illegal(group)
- return posixpath.split(group)
+ i = group.rfind(SEPERATOR) + 1
+ head, tail = group[:i], group[i:]
+ head = head.rstrip(SEPERATOR)
+
+ return head, tail
def commonprefix(args):
@@ -113,26 +117,50 @@ def commonprefix(args):
for group in args:
_assert_illegal(group)
- return posixpath.commonprefix(args)
+ if len(args) == 1:
+ return args
+ common = []
-def join(*args):
+ for elems in zip(*[split(a) for a in args]):
+ iter_ = iter(elems)
+ first = next(iter_)
+ if all(first == r for r in iter_):
+ common.append(first)
+ else:
+ break
+
+ return join(*common)
+
+
+def join(first, *args):
"""Join multiple groups together with some sanity checking.
Prevents groups from having '/' as the leading character or from having
'\\' in them, as these are groups not paths.
+ This function is implemented via string concatenation, while most
+ pythonistas would use list joining, because it is accepted as better. I
+ wrote a number of implementations and timed them with timeit. I found for
+ small number of joins (2-10) that str concatenation was quite a bit faster,
+ at around 100 elements list joining became faster. Since most of piglit's
+ use of join is for 2-10 elements I used string concatentation, which is
+ conincedently very similar to the way posixpath.join is implemented.
+
"""
args = [_normalize(group) for group in args]
+ first = _normalize(first)
+ _assert_illegal(first)
for group in args:
- assert isinstance(group, (str, unicode)), \
- 'Type must be string or unicode'
- assert '\\' not in group, \
- 'Groups are not paths and cannot contain \\. ({})'.format(group)
- assert not args[0].startswith('/'), \
- 'Groups cannot start with /. ({})' .format(args[0])
+ _assert_illegal(group)
+ # Only append things if the group is not empty, otherwise we'll get
+ # extra SEPERATORs where we don't want them
+ if group:
+ if not first.endswith(SEPERATOR):
+ first += SEPERATOR
+ first += group
- return posixpath.join(*args)
+ return first
def split(group):
@@ -145,7 +173,7 @@ def split(group):
_assert_illegal(group)
if group == '':
return []
- return group.split('/')
+ return group.split(SEPERATOR)
def from_path(path):
@@ -157,13 +185,11 @@ def from_path(path):
"""
assert isinstance(path, (str, unicode)), 'Type must be string or unicode'
- assert not path.startswith('/'), \
- 'Groups cannot start with /. ({})' .format(path)
if '\\' in path:
- return path.replace('\\', '/')
-
+ path = path.replace('\\', SEPERATOR)
+ if '/' in path:
+ path = path.replace('/', SEPERATOR)
if '.' == path:
return ''
-
return path
diff --git a/framework/tests/backends_tests.py b/framework/tests/backends_tests.py
index 5b3b10c..8916ea5 100644
--- a/framework/tests/backends_tests.py
+++ b/framework/tests/backends_tests.py
@@ -48,6 +48,8 @@ BACKEND_INITIAL_META = {
JUNIT_SCHEMA = 'framework/tests/schema/junit-7.xsd'
+doc_formatter = utils.DocFormatter({'seperator': grouptools.SEPERATOR})
+
def test_initialize_jsonbackend():
""" Test that JSONBackend initializes
@@ -168,6 +170,30 @@ class TestJUnitMultiTest(TestJUnitSingleTest):
super(TestJUnitMultiTest, self).test_xml_valid()
+ at doc_formatter
+def test_junit_replace():
+ """JUnitBackend.write_test: '{seperator}' is replaced with '.'"""
+ with utils.tempdir() as tdir:
+ test = backends.JUnitBackend(tdir)
+ test.initialize(BACKEND_INITIAL_META)
+ test.write_test(
+ grouptools.join('a', 'test', 'group', 'test1'),
+ results.TestResult({
+ 'time': 1.2345,
+ 'result': 'pass',
+ 'out': 'this is stdout',
+ 'err': 'this is stderr',
+ 'command': 'foo',
+ })
+ )
+ test.finalize()
+
+ test_value = etree.parse(os.path.join(tdir, 'results.xml')).getroot()
+
+ nt.assert_equal(test_value.find('.//testcase').attrib['classname'],
+ 'piglit.a.test.group')
+
+
def test_json_initialize_metadata():
""" JSONBackend.initialize() produces a metadata.json file """
with utils.tempdir() as f:
diff --git a/framework/tests/grouptools_tests.py b/framework/tests/grouptools_tests.py
index a6a89a5..a8b55fb 100644
--- a/framework/tests/grouptools_tests.py
+++ b/framework/tests/grouptools_tests.py
@@ -27,6 +27,10 @@ import nose.tools as nt
import framework.grouptools as grouptools
import framework.tests.utils as utils
+doc_formatter = utils.DocFormatter({ # pylint: disable=invalid-name
+ 'seperator': grouptools.SEPERATOR,
+})
+
@utils.nose_generator
def generate_tests():
@@ -59,7 +63,15 @@ def generate_tests():
def test_grouptools_join():
"""grouptools.join: works correctly."""
# XXX: this hardcoded / needs to be fixed
- nt.assert_equal(grouptools.join('g1', 'g2'), 'g1/g2')
+ nt.assert_equal(grouptools.join('g1', 'g2'),
+ grouptools.SEPERATOR.join(['g1', 'g2']))
+
+
+ at doc_formatter
+def test_grouptools_join_notrail():
+ """grouptools.join: doesn't add trailing {seperator} with empty element"""
+ test = grouptools.join('g1', 'g2', '')
+ nt.ok_(not test.endswith(grouptools.SEPERATOR), msg=test)
def test_split_input_empty():
@@ -67,14 +79,19 @@ def test_split_input_empty():
nt.assert_equal(grouptools.split(''), [])
+ at doc_formatter
def test_from_path_posix():
- """grouptools.from_path: doesn't change posixpaths."""
+ """grouptools.from_path: converts / to {seperator} in posix paths."""
+ # Since we already have tests for grouptools.join we can trust it to do the
+ # right thing here. This also means that the test doesn't need to be
+ # updated if the separator is changed.
nt.assert_equal(grouptools.from_path('foo/bar'),
grouptools.join('foo', 'bar'))
+ at doc_formatter
def test_from_path_nt():
- """grouptools.from_path: converts \\ to / in nt paths."""
+ """grouptools.from_path: converts \\ to {seperator} in nt paths."""
nt.assert_equal(grouptools.from_path('foo\\bar'),
grouptools.join('foo', 'bar'))
diff --git a/framework/tests/utils.py b/framework/tests/utils.py
index dc8af64..3970659 100644
--- a/framework/tests/utils.py
+++ b/framework/tests/utils.py
@@ -302,3 +302,41 @@ def no_error(func):
raise TestFailure(e.message)
return test_wrapper
+
+
+class DocFormatter(object):
+ """Decorator for formatting object docstrings.
+
+ This class is designed to be initialized once per test module, and then one
+ instance used as a decorator for all functions.
+
+ Works as follows:
+ >>> doc_formatter = DocFormatter({'format': 'foo', 'name': 'bar'})
+ >>>
+ >>> @doc_formatter
+ ... def foo():
+ ... '''a docstring for {format} and {name}'''
+ ... pass
+ ...
+ >>> foo.__doc__
+ 'a docstring for foo and bar'
+
+ This allows tests that can be dynamically updated by changing a single
+ constant to have the test descriptions alos updated by the same constant.
+
+ Arguments:
+ table -- a dictionary of key value pairs to be converted
+
+ """
+ def __init__(self, table):
+ self.__table = table
+
+ def __call__(self, func):
+ try:
+ func.__doc__ = func.__doc__.format(**self.__table)
+ except KeyError as e:
+ # We want to catch this to ensure that a test that is looking for a
+ # KeyError doesn't pass when it shouldn't
+ raise UtilsError(e)
+
+ return func
--
2.3.1
More information about the Piglit
mailing list