On Wed, Feb 20, 2013 at 11:41 PM, Ben Reser <ben_at_reser.org> wrote:
> 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.
>
Will fix..
>
> 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.
>
The function here is a boolean function, so I assumed that checking for int
(0/1) would be sufficient. What I comprehend is that there is a need for me
to add a check to if those values being returned are 0 or 1.
> > + #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.
>
What my instant intuition says is that for the svn_checksum_t* to be a
duplicate of one another the kind, digest length (since the digest value
itself will not be the same), pool of the respective objects needs to be
the same.
Would that be a correct approach??
A possible stub could be :
*if ( val.kind is duplicate.kind ) and (val.digest_len is
duplicate.digest_len) -> then they are duplicates.*
Since the pool allocated remains fixed here.
Also there is a mismatch in the digest.this for the 2 svn_checksum_t
objects val and duplicate, will attributing this to the different memory
locations of (digest.this) be the correct thing to do??
Matching the length of the digest here doesn't exactly seem the best
approach to me here ..:\
Received on 2013-02-21 01:02:00 CET