Re: [PATCH] Improve svn_checksum_t bindings in SWIG

Yeah, you're right. Ultimately that's because svn_checksum_size takes
a checksum rather than a checksum kind.

What should we do then?

- revv svn_checksum_size to take an svn_checksum_kind_t?

- svn.core.APR_MD5_DIGESTSIZE? svn.core doesn't export that symbol.

- len(hashlib.md5().hexdigest()) ?

- 32 ?

- explicitly test the digest of a known string (even the C unit tests
  don't do that)?

In the meantime, I applied Shivani's patch in r1420334.

Peter Samuelson wrote on Tue, Dec 11, 2012 at 12:49:27 -0600:
>
> > > +LENGTH = svn.core.svn_checksum_size(svn.core.svn_checksum_create(svn.core.svn_checksum_md5))
>
> > > + self.assertEqual(len(check_val)%LENGTH,0,"Length of digest does not match kind")
>
> Is there a better way to get the expected length?
> svn.core.svn_checksum_create(svn.core.svn_checksum_md5) is called twice
> here. If we had an off-by-one bug in the length of the object returned
> from svn_checksum_create, this test wouldn't catch it.
>
> I know others have said not to hardcode a 32 here. But you're already
> hardcoding MD5. I'd say if there's no other convenient alternative,
> hardcoding the knowledge that "md5 is 128 bits" (32 hex digits) seems
> better than testing that two objects created by the same constructor
> are the same length.
>
> I mean, to extend this test to other checksum kinds would require
> something of a rewrite anyway.
Received on 2012-12-11 20:12:07 CET