On Tue, Feb 19, 2013 at 10:01 PM, Shivani Poddar
<shivani.poddar92_at_gmail.com> wrote:
>
> [ [ [
>
> Improving support for svn_checksum.h in SWIG bindings.
Same comment as last time about the description line.
> * subversion/bindings/swig/python/tests/checksum.py: "swig-py: Adding tests
> for
>
> svn_checksum_dup() and svn_checksum_match()"
This isn't properly formatted per our commit log style. We don't use
quotes and we don't leave blank lines.
> Index: subversion/bindings/swig/python/tests/checksum.py
> ===================================================================
> --- subversion/bindings/swig/python/tests/checksum.py (revision 1448005)
> +++ subversion/bindings/swig/python/tests/checksum.py (working copy)
> @@ -27,15 +27,20 @@
> # function
> kind, expected_length = svn.core.svn_checksum_md5, 128/8
> val = svn.core.svn_checksum_create(kind)
> + duplicate = svn.core.svn_checksum_dup(val)
> check_val = svn.core.svn_checksum_to_cstring_display(val)
> -
> + #create manual duplicate of val to check svn_checksum_match()
> + dup_val = val
> self.assertTrue(isinstance(check_val, str),
> "Type of digest not string")
> self.assertEqual(len(check_val), 2*expected_length,
> "Length of digest does not match kind")
> self.assertEqual(int(check_val), 0,
> "Value of initialized digest is not 0")
> -
> + self.assertTrue(isinstance(svn.core.svn_checksum_match(val,dup_val), int),"Type returned is not int(0/1) or boolean")
Don't abuse line wrapping to deal with overly wrong lines, just
continue the code on the next line and use spaces to shift it to an
appropriate indentation.
Your description and the test you're running don't equate. You aren't
checking that it is a boolean or that if it is an int is only 0 or 1.
Somewhat surprised that the Python don't seem to be mapping our
svn_boolean_t to the Python boolean values but that's not really your
issue.
> + #check for the function svn_checksum_dup() independently
> + self.assertTrue(svn.core.svn_checksum_match(val,duplicate),
> + "Argument 2 is not a duplicate of argument 1")
> def suite():
> return unittest.defaultTestLoader.loadTestsFromTestCase(ChecksumTestCases)
I'd say that your description for this assert could be more useful.
svn_checksum_match() probably doesn't do what you think it does. It
treats an all zero checksum as always matching as long as the kinds
are the same. So you can't use it to make sure that
svn_checksum_dup() works especially when you're only creating
checksums with create() that always returns all zero checksums. Let's
say that dup() is not working for some reason and it returns a
checksum that has a different value, the match() function would still
return true since your original checksum had all zeros.
See the documentation here:
http://people.apache.org/~hwright/svn/doc/api/trunk/svn__checksum_8h.html#a417244d5739a22f9a18e440f4f43d12d
I think this is a good point to start accessing the values in the
svn_checksum_t object directly.
Received on 2013-02-20 19:11:53 CET