So, I did a full run before I pushed this, and discovered an interesting issue.
Some test have '/' in their names. This makes a lot of interesting problems. For one, the update pass I've written is wrong, since it naively replaces all '/' and '\\' with '@', which means that tests could end up with different names before and after. It also means that the asserts here are not safe to use. I guess I'll send out a v2 later today when I decide what the right way to handle this is. Dylan On Thu, Mar 12, 2015 at 03:42:10PM -0700, Dylan Baker wrote: > This removes the _normalize function and adds an assertion for \\ or for > / > > It also adds some tests for the assertions in grouptools. > > Signed-off-by: Dylan Baker <[email protected]> > --- > framework/grouptools.py | 27 +++------------------- > framework/tests/grouptools_tests.py | 46 > +++++++++++++++++++++++++++++++++++++ > 2 files changed, 49 insertions(+), 24 deletions(-) > > diff --git a/framework/grouptools.py b/framework/grouptools.py > index abe52cf..d6fc921 100644 > --- a/framework/grouptools.py > +++ b/framework/grouptools.py > @@ -29,8 +29,6 @@ posix paths they may not start with a leading '/'. > > """ > > -import os.path > - > __all__ = [ > 'commonprefix', > 'from_path', > @@ -44,27 +42,14 @@ __all__ = [ > SEPERATOR = '@' > > > -def _normalize(group): > - """Helper to normalize group paths on Windows. > - > - Although grouptools' heart is in the right place, the fact is that it > fails > - to spot when developers mistakedly use os.path.join for groups on Posix > - systems. > - > - So until this is improved somehow, make grouptools behavior on Windows > - match Linux, ie, just silently ignore mixed use of grouptools and > os.path. > - """ > - if os.path.sep != '/': > - group = group.replace(os.path.sep, '/') > - return group > - > - > def _assert_illegal(group): > """Helper that checks for illegal characters in input.""" > 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 in group, \ > + 'Groups are not paths and cannot contain /. ({})'.format(group) > > > def testname(group): > @@ -77,7 +62,6 @@ def testname(group): > Analogous to os.path.basename > > """ > - group = _normalize(group) > _assert_illegal(group) > > return splitname(group)[1] > @@ -93,7 +77,6 @@ def groupname(group): > Analogous to os.path.dirname > > """ > - group = _normalize(group) > _assert_illegal(group) > > return splitname(group)[0] > @@ -101,7 +84,6 @@ def groupname(group): > > def splitname(group): > """Split a group name, Returns tuple "(group, test)".""" > - group = _normalize(group) > _assert_illegal(group) > > i = group.rfind(SEPERATOR) + 1 > @@ -113,7 +95,6 @@ def splitname(group): > > def commonprefix(args): > """Given a list of groups, returns the longest common leading > component.""" > - args = [_normalize(group) for group in args] > for group in args: > _assert_illegal(group) > > @@ -148,11 +129,10 @@ def join(first, *args): > 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_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: > @@ -169,7 +149,6 @@ def split(group): > If input is '' return an empty list > > """ > - group = _normalize(group) > _assert_illegal(group) > if group == '': > return [] > diff --git a/framework/tests/grouptools_tests.py > b/framework/tests/grouptools_tests.py > index a8b55fb..5377749 100644 > --- a/framework/tests/grouptools_tests.py > +++ b/framework/tests/grouptools_tests.py > @@ -60,6 +60,52 @@ def generate_tests(): > yield test, func, args, expect > > > [email protected]_generator > +def generate_error_tests(): > + """Generate tests for the various failure problems.""" > + > + @nt.raises(AssertionError) > + def test(func, args): > + """The input must be either a str or unicode.""" > + func(args) > + > + tests = [ > + ('testname', grouptools.testname), > + ('groupname', grouptools.groupname), > + ('splitname', grouptools.splitname), > + ('split', grouptools.split), > + ('join', grouptools.join), > + ] > + > + for name, func in tests: > + test.description = \ > + 'grouptools.{}: raises an error if args are invalid'.format(name) > + # This must be a list of lists to trigger commonprefix > + yield test, func, ['foo'] > + > + test.description = \ > + 'grouptools.{}: raises an error if / is in the args'.format(name) > + yield test, func, 'foo/bar' > + > + test.description = \ > + 'grouptools.{}: raises an error if \\ is in the > args'.format(name) > + yield test, func, 'foo\\bar' > + > + # commonprefix takes a list of things rather than things, so it must be > + # tested separately > + test.description = \ > + 'grouptools.commonprefix: raises an error if args are invalid' > + yield test, grouptools.commonprefix, [['foo']] > + > + test.description = \ > + 'grouptools.commonprefix: raises an error if / is in args' > + yield test, grouptools.commonprefix, ['foo/bar', 'boink'] > + > + test.description = \ > + 'grouptools.commonprefix: raises an error if \\ is in args' > + yield test, grouptools.commonprefix, ['foo\\bar', 'boink'] > + > + > def test_grouptools_join(): > """grouptools.join: works correctly.""" > # XXX: this hardcoded / needs to be fixed > -- > 2.3.1 >
signature.asc
Description: Digital signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
