On Thu, Feb 21, 2013 at 5:31 AM, Shivani Poddar
<shivani.poddar92_at_gmail.com>wrote:
>
>
> 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 ..:\
>
>
In light of the feedback given above,
The best possible solution to me is to match the digest strings for the
both the svn_checksum_t objects.
But even so the svn_checksum_create() only gives me digests intialized to 0
, meaning that whatever objects I have will match with the given test. Is
there any suggestion as to would doing this anyways be right??
Received on 2013-03-12 11:09:08 CET