On Wed, Dec 12, 2012 at 12:40 AM, Daniel Shahaf <danie...@elego.de> wrote:
> 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? > Does this line mean something like: LENGTH = svn.core.svn_checksum_size(svn.core.svn_checksum_create(svn.core.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. > -- Shivani Poddar, Bachelors in Computer Sciences and MS in Exact Humanities, Sophomore International Institute of Information Technology, Hyderabad