[Piglit] [PATCH 01/10] grouptools.py: Add a module specifically for working with group strings

Fri Dec 5 03:55:41 PST 2014

On 04/12/14 23:09, Dylan Baker wrote:
> This module is largely just posixpath (sometimes the functions are
> renamed), with a couple of unique functions, which are basically
> wrappers around posixpath functions with some special exceptions. Having
> and using the module presents the advantage of being able to change the
> implementation, including how the groups are manipulated. It also is
> clearer than working with posixpath directly.
>
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
> ---
>   framework/grouptools.py             | 151 +++++++++++++++++++++++++++++++
>   framework/tests/grouptools_tests.py | 174 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 325 insertions(+)
>   create mode 100644 framework/grouptools.py
>   create mode 100644 framework/tests/grouptools_tests.py
>
> diff --git a/framework/grouptools.py b/framework/grouptools.py
> new file mode 100644
> index 0000000..64e08b7
> --- /dev/null
> +++ b/framework/grouptools.py
> @@ -0,0 +1,151 @@
> +# Copyright (c) 2014 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
> +# in the Software without restriction, including without limitation the rights
> +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> +# copies of the Software, and to permit persons to whom the Software is
> +# furnished to do so, subject to the following conditions:
> +
> +# The above copyright notice and this permission notice shall be included in
> +# all copies or substantial portions of the Software.
> +
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> +# SOFTWARE.
> +
> +"""Module providing utility functions to work with piglit groups.
> +
> +Instead of using posixpath (or the generic os.path) for working with tests this
> +module should be prefered.
> +
> +Piglit groups look much like posix paths, they are '/' delimited with each
> +element representing a group, and the final element being the test name. Unlike
> +posix paths they may not start with a leading '/'.
> +
> +"""
> +
> +import posixpath
> +
> +__all__ = [
> +    'join',
> +    'commonprefix',
> +    'relgroup',
> +    'split',
> +    'groupname',
> +    'testname',
> +    'splitname',
> +    'from_path'
> +]
> +
> +
> +def testname(group):
> +    """Return the last element of a group name.
> +
> +    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.
> +
> +    Analogous to os.path.basename
> +
> +    """
> +    assert '\\' not in group, 'Groups are not paths and cannot contain \\'
> +    assert not group.startswith('/'), 'Groups cannot start with /'
> +
> +    return posixpath.basename(group)
> +
> +
> +def groupname(group):
> +    """Return all groups except the last.
> +
> +    Provided the value 'group1/group2/test1' will provide 'group1/group2', this
> +    does not enforce any rules that the final element is a test name, and can
> +    be used to shaved down groups.
> +
> +    Analogous to os.path.dirname
> +
> +    """
> +    assert '\\' not in group, 'Groups are not paths and cannot contain \\'
> +    assert not group.startswith('/'), 'Groups cannot start with /'
> +
> +    return posixpath.dirname(group)
> +
> +
> +def splitname(group):
> +    """Split a group name, Returns tuple "(group, test)"."""
> +    assert '\\' not in group, 'Groups are not paths and cannot contain \\'

I think the series is a good cleanup.

But have you run any tests on Windows? There are so many places that 
could introduce '\\' in the paths, so I'm concerned this can leave 
Windows totally broken.

I can test this myself if you want. Just please push the series to some 
repos I can pull from.

Jose

> +    assert not group.startswith('/'), 'Groups cannot start with /'
> +
> +    return posixpath.split(group)
> +
> +
> +def commonprefix(args):
> +    """Given a list of groups, returns the longest common leading component."""
> +    for group in args:
> +        assert '\\' not in group, 'Groups are not paths and cannot contain \\'
> +        assert not group.startswith('/'), 'Groups cannot start with /'
> +
> +    return posixpath.commonprefix(args)
> +
> +
> +def join(*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.
> +
> +    """
> +    for group in args:
> +        assert '\\' not in group, 'Groups are not paths and cannot contain \\'
> +    assert not args[0].startswith('/'), 'Groups cannot start with /'
> +
> +    return posixpath.join(*args)
> +
> +
> +def relgroup(large, small):
> +    """Find the relationship between two groups.
> +
> +    This allows the comparison of two groups, and returns a string. If start
> +    start is longer than the group then '' is returned.
> +
> +    """
> +    for element in {large, small}:
> +        assert '\\' not in element, 'Groups are not paths and cannot contain \\'
> +        assert not element.startswith('/'), 'Groups cannot start with /'
> +
> +    if len(small) > len(large):
> +        return ''
> +    elif small == '' and large == '':
> +        return ''
> +    else:
> +        return posixpath.relpath(large, small)
> +
> +
> +def split(group):
> +    """Split the group into a list of elements.
> +
> +    If input is '' return an empty list
> +
> +    """
> +    assert '\\' not in group, 'Groups are not paths and cannot contain \\'
> +    assert not group.startswith('/'), 'Groups cannot start with /'
> +    if group == '':
> +        return []
> +    return group.split('/')
> +
> +
> +def from_path(path):
> +    """Create a group from a path.
> +
> +    This function takes a path, and creates a group out of it.
> +
> +    This safely handles both Windows and Unix style paths.
> +
> +    """
> +    if '\\' in path:
> +        return path.replace('\\', '/')
> +    return path
> diff --git a/framework/tests/grouptools_tests.py b/framework/tests/grouptools_tests.py
> new file mode 100644
> index 0000000..f65ad10
> --- /dev/null
> +++ b/framework/tests/grouptools_tests.py
> @@ -0,0 +1,174 @@
> +# Copyright (c) 2014 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
> +# in the Software without restriction, including without limitation the rights
> +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> +# copies of the Software, and to permit persons to whom the Software is
> +# furnished to do so, subject to the following conditions:
> +
> +# The above copyright notice and this permission notice shall be included in
> +# all copies or substantial portions of the Software.
> +
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> +# SOFTWARE.
> +
> +"""Module with tests for grouptools."""
> +
> +from __future__ import print_function
> +import inspect
> +
> +import nose.tools as nt
> +
> +import framework.grouptools as grouptools
> +import framework.tests.utils as utils
> +
> +
> +def description(desc):
> +    """Set description on a bound method.
> +
> +    This is an ugly little mother that does something awful, it sets the
> +    description attribute on a bound method. This allows a bound method to be
> +    passed from a test generator with a description.
> +
> +    The string will be formated passing self.__dict__ to str.vformat, so any
> +    values that should be filled in should have the same names as the
> +    attributes of the class
> +
> +    """
> +    def wrapper(func):
> +        func.description = desc
> +        return func
> +    return wrapper
> +
> +
> +class _SharedFunctionTest(object):  # pylint: disable=too-few-public-methods
> +    """Magic test class."""
> +    def __init__(self, name, function, input_, expected, explode=False):
> +        # pylint: disable=too-many-arguments
> +        self.input = input_
> +        self.expected = expected
> +        self.name = name
> +        self.function = function
> +        self.explode = explode
> +
> +    def __iter__(self):
> +        """Iterate over the test methods of this class.
> +
> +        Like the description wrapper this is ugly, it iterates over the bound
> +        methods of the class looking for those who's names start with test_,
> +        then it uses getatter to get the coresponding method, then formats that
> +        description string of that method with vformat, and finally yields the
> +        test.
> +
> +        """
> +        # Not exactly sure why x() needs to be called, but it does.
> +        wrapper = lambda x: x()
> +
> +        for name, test in inspect.getmembers(self, inspect.ismethod):
> +            if name.startswith('test_'):
> +                wrapper.description = test.description.format(**self.__dict__)
> +                yield wrapper, test
> +
> +
> +class _GroupToolsTest(_SharedFunctionTest):
> +    """Not so simple class for running the same tests on multiple callables.
> +
> +    Provides a set of test methods that rely on instance attributes to do the
> +    testing, meaning setting a few attributes creates a complete test.
> +
> +    Arguments:
> +    input --    data to pass to the function
> +    expected -- the result that is expected
> +    name --     the name of the function being tested
> +    function -- the function to test
> +    explode --  if the function takes multiple arguments they must be passed as
> +                container, if explode is set to True then the container will be
> +                exploded when passed in
> +
> +    """
> +    @description("grouptools.{name}: works")
> +    def test_functionality(self):
> +        """Test that the functionality of the function."""
> +        if not self.explode:
> +            nt.assert_equal(self.function(self.input), self.expected)
> +        else:
> +            nt.assert_equal(self.function(*self.input), self.expected)
> +
> +    @description("grouptools.{name}: doesn't accept a leading /")
> +    @nt.raises(AssertionError)
> +    def test_assertion_slash(self):
> +        """Test that a leading / is an error."""
> +        if isinstance(self.input, (str, unicode)):
> +            self.function('/' + self.input)
> +        elif not self.explode:
> +            self.function(['/' + i for i in self.input])
> +        else:
> +            self.function(*['/' + i for i in self.input])
> +
> +    @description("grouptools.{name}: doesn't accept \\ in groups")
> +    @nt.raises(AssertionError)
> +    def test_assertion_backslash(self):
> +        """Test that \\ in a group is an error."""
> +        if isinstance(self.input, (str, unicode)):
> +            self.function(self.input.replace('/', '\\'))
> +        elif not self.explode:
> +            self.function(i.replace('/', '\\') for i in self.input)
> +        else:
> +            self.function(*[i.replace('/', '\\') for i in self.input])
> +
> +
> + at utils.nose_generator
> +def generate_tests():
> +    """Generate tests for the groups tools module.
> +
> +    This cannot test all corners of the more complicated members.
> +
> +    """
> +    # pylint: disable=line-too-long
> +    tests = [
> +        ('testname', grouptools.testname, 'g1/g2/t1', 't1'),
> +        ('groupname', grouptools.groupname, 'g1/g2/t1', 'g1/g2'),
> +        ('splitname', grouptools.splitname, 'g1/g2/t1', ('g1/g2', 't1')),
> +        ('commonprefix', grouptools.commonprefix, ['g1/g2/1', 'g1/g2/2'], 'g1/g2/'),
> +        ('join', grouptools.join, ['g1/g2', 't1'], 'g1/g2/t1', True),
> +        ('split', grouptools.split, 'g1/g2/t1', ['g1', 'g2', 't1']),
> +        ('relgroup', grouptools.relgroup, ['g1/g2/g3/t1', 'g1/g2'], 'g3/t1', True)
> +    ]
> +    # pylint: enable=line-too-long
> +
> +    for args in tests:
> +        test_class = _GroupToolsTest(*args)
> +        # In python3 we could use 'yield from'
> +        for wrapper, test in test_class:
> +            yield wrapper, test
> +
> +
> +def test_relgroup_small_gt_large():
> +    """grouptools.relgroup: if small > large return ''."""
> +    nt.assert_equal(grouptools.relgroup('foo', 'foobar'), '')
> +
> +
> +def test_relgroup_both_empty():
> +    """grouptools.relgroup: if small == '' and large == '' return ''."""
> +    nt.assert_equal(grouptools.relgroup('', ''), '')
> +
> +
> +def test_split_input_empty():
> +    """grouptools.split: an empty input returns []."""
> +    nt.assert_equal(grouptools.split(''), [])
> +
> +
> +def test_from_path_posix():
> +    """grouptools.from_path: doesn't change posixpaths."""
> +    nt.assert_equal(grouptools.from_path('foo/bar'), 'foo/bar')
> +
> +
> +def test_from_path_nt():
> +    """grouptools.from_path: converts \\ to / in nt paths."""
> +    nt.assert_equal(grouptools.from_path('foo\\bar'), 'foo/bar')
>