On Wed, Dec 12, 2012 at 12:02 AM, Daniel Shahaf <danie...@elego.de> wrote:

> Shivani Poddar wrote on Tue, Dec 11, 2012 at 23:49:56 +0530:
> > Log Message:
> >
> > Improve support for svn_checksum.h in SWIG bindings
> > * subversion/bindings/swig/python/tests/checksum.py: Improved
> test_checksum
>
> You haven't fixed the log message as per my original review.
>
> > 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,23 +20,18 @@
> >  #
> >  import unittest, setup_path
> >  import svn.core
> > -
> > +LENGTH =
> svn.core.svn_checksum_size(svn.core.svn_checksum_create(svn.core.svn_checksum_md5))
> >  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)
> > +
> > +        self.assertEqual(type(check_val),str,"Type of digest not
> string")
>
> assertIsInstance() would be more appropriate, but that's not critical.
>
> > +        self.assertEqual(len(check_val)%LENGTH,0,"Length of digest does
> not match kind")
>
> Why module?  == 2*LENGTH is fine.  (Two hexdigits per byte.)
>

Module seemed to me as a more intuitive choice although yes even 2*LENGTH
would do..


>
> > +        self.assertEqual(int(check_val),0,"Value of initialized digest
> is not 0")
> >
> >  def suite():
> >      return
> unittest.defaultTestLoader.loadTestsFromTestCase(ChecksumTestCases)
> >  if __name__ == '__main__':
> > @@ -45,4 +40,3 @@
> >
> >
> >
> > -
>
> Gratuitous whitespace change.
>
> Anyway, I'll commit the patch in a few minutes.
>



-- 
Shivani Poddar,
Bachelors in Computer Sciences and MS in Exact Humanities, Sophomore
International Institute of Information Technology, Hyderabad

Reply via email to