[MERGE] Make knits the default format

Martin Pool mbp at sourcefrog.net
Thu Apr 20 02:48:05 BST 2006


On 20 Apr 2006, Robert Collins <robertc at robertcollins.net> wrote:
> Again, theres more than 'just toggle the default' - because if there
> isn't, tests dont pass.

This will refuse to load any existing experimental knits - people will
have to re-convert any branches they may have made.

+1 with some fixes.

> === modified file 'a/bzrlib/errors.py'
> --- a/bzrlib/errors.py	
> +++ b/bzrlib/errors.py	
> @@ -764,6 +764,14 @@
>      """Parameter $(param)s is required but not present."""
>  
>  
> +class BzrBadParameterUnicode(BzrBadParameter):
> +    """Parameter %(param) is unicode by only byte-strings are permitted."""
> +
> +

Remember these two must be %(param)s - you missed the 's'

That message is confusing - do you mean "but only"?

> +class BzrBadParameterContainsNewline(BzrBadParameter):
> +    """Parameter %(param) contains a newline."""
> +
> +
>  class DependencyNotPresent(BzrNewError):
>      """Unable to import library "%(library)s": %(error)s"""
>  
> 

> === modified file 'a/bzrlib/tests/__init__.py'
> --- a/bzrlib/tests/__init__.py	
> +++ b/bzrlib/tests/__init__.py	
> @@ -341,7 +341,8 @@
>      def assertIsInstance(self, obj, kls):
>          """Fail if obj is not an instance of kls"""
>          if not isinstance(obj, kls):
> -            self.fail("%r is not an instance of %s" % (obj, kls))
> +            self.fail("%r is an instance of %s rather than %s" % (
> +                obj, obj.__class__, kls))

Is it safe to do __class__ on any object?  It seems to work even for
None or other primitives.

> === modified file 'a/bzrlib/tests/revisionstore_implementations/test_all.py'
> --- a/bzrlib/tests/revisionstore_implementations/test_all.py	
> +++ b/bzrlib/tests/revisionstore_implementations/test_all.py	
> @@ -107,7 +107,9 @@
>          # we get a revision count and a numeric size figure from total_size().
>          count, bytes = self.store.total_size(self.transaction)
>          self.assertEqual(0, count)
> -        self.assertEqual(0, bytes)
> +        # some stores use disk immediately that they are created so we just 
> +        # check that its an int.
> +        self.assertIsInstance(bytes, (int, long))
>          self.add_sample_rev()
>          count, bytes = self.store.total_size(self.transaction)
>          self.assertEqual(1, count)

This test is a bit deceptive - it looks like it checks they are an int
and a long, but it actually just checks its a tuple.  Being able to
check that something matches such a template might be nice, but
assertIsInstance isn't that method.  For clarity, either check them
separately or just check it's a tuple.

> === modified file 'a/bzrlib/tests/test_osutils.py'
> --- a/bzrlib/tests/test_osutils.py	
> +++ b/bzrlib/tests/test_osutils.py	
> @@ -90,3 +90,16 @@
>          self.assertRaises(BzrBadParameterNotUnicode,
>                            osutils.safe_unicode,
>                            '\xbb\xbb')
> +
> +
> +class TestSplitLines(TestCase):
> +
> +    def test_split_unicode(self):
> +        self.assertEqual([u'foo\n', u'bar\xae'],
> +                         osutils.split_lines(u'foo\nbar\xae'))
> +        self.assertEqual([u'foo\n', u'bar\xae\n'],
> +                         osutils.split_lines(u'foo\nbar\xae\n'))
> +
> +    def test_split_with_carriage_returns(self):
> +        self.assertEqual(['foo\rbar\n'],
> +                         osutils.split_lines('foo\rbar\n'))


Would be good to also test we can split '', '\n', etc.

-- 
Martin




More information about the bazaar mailing list