Shivani Poddar wrote on Tue, Dec 11, 2012 at 02:22:28 +0530:
> Log Message:
>
> Improve support for svn_checksum.h in SWIG bindings
> * subversion/bindings/swig/python/tests/checksum.py: Improved test_checksum
>
Need a blank line before the * line, and to use the "* file\n (symbol)"
syntax --- 'test_checksum' is a symbol.
> Review by: danielsh
>
That's inappropriate; I haven't reviewed the patch yet. You might add
this field after I review it, not before.
> Index: subversion/bindings/swig/python/tests/checksum.py
> ===================================================================
> --- subversion/bindings/swig/python/tests/checksum.py (revision 1419694)
> +++ subversion/bindings/swig/python/tests/checksum.py (working copy)
> @@ -20,22 +20,25 @@
> #
> import unittest, setup_path
> import svn.core
> -
> +LENGTH = 32 or 40;
This is wrong in two different ways:
- "32 or 40" is a constant expression that evaluates to 32.
- You hardcode two values, when you should be hardcoding neither of
them. (The bindings shouldn't need to change if the C library grows
a third svn_checksum_kind_t.)
> class ChecksumTestCases(unittest.TestCase):
> def test_checksum(self):
> # Checking primarily the return type for the svn_checksum_create
> # function
> val = svn.core.svn_checksum_create(svn.core.svn_checksum_md5)
> check_val = svn.core.svn_checksum_to_cstring_display(val)
> -
> # The svn_checksum_to_cstring_display should return a str type object
> # from the check_val object passed to it
> if(type(check_val) == str):
> - # The intialized value created from a checksum should be 0
> - if(int(check_val) != 0):
> - self.assertRaises(AssertionError)
> + #Check the length of the string check_val
> + if(len(check_val) == LENGTH):
> + # The intialized value created from a checksum should be 0
> + if(int(check_val) != 0):
> + raise
This bare "raise" statement without arguments is itself an error.
See for yourself:
% python -c 'raise'
TypeError: exceptions must be old-style classes or derived from BaseException, not NoneType
This exception signifies a bug in your program. It has become
a RuntimeError in recent Pythons (and, frankly, could become
a compile-time error as well --- the compiler knows there's no except:
block surrounding this statement). It might work, but not because it's
correct.
Daniel
> + else:
> + raise
> else:
> - self.assertRaises(TypeError, test_checksum)
> + raise
>
> def suite():
> return unittest.defaultTestLoader.loadTestsFromTestCase(ChecksumTestCases)
Received on 2012-12-10 23:39:17 CET