[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Improve svn_checksum_t bindings in SWIG

From: Ben Reser <ben_at_reser.org>
Date: Tue, 11 Dec 2012 09:18:03 -0800

On Tue, Dec 11, 2012 at 3:18 AM, Shivani Poddar
<shivani.poddar92_at_gmail.com> wrote:
> Log Message:
> Improve support for svn_checksum.h in SWIG bindings
> * subversion/bindings/swig/python/tests/checksum.py: Improved test_checksum
> Modified:
> subversion/trunk/subversion/bindings/swig/python/tests/checksum.py

This doesn't even pass. You really should run your code before submitting it.
ERROR: test_checksum (checksum.ChecksumTestCases)
Traceback (most recent call last):
  File "/home/breser/wandisco/share/wcs/svn-trunk/subversion/bindings/swig/python/tests/checksum.py",
line 39, in test_checksum
    raise Exception ("Length of Initialized digest does not match kind")
Exception: Length of Initialized digest does not match kind

This doesn't provide a LENGTH:
+LENGTH = svn.core.svn_checksum_to_cstring_display(svn.core.svn_checksum_create(svn.core.svn_checksum_md5))

Before you come back with an adjustment to just fix that... I urge
you to consider what you're testing here. You can't just call the
same functions twice and make sure the output matches. That doesn't
result in a very useful test.

My suggestion would be to write a function inside your test function
that determines if the result is the proper size. There is a function
in svn_checksum_* that provides you with the size of a digest. You
might want to read the C code for it to get an idea of how it works
and see if it's useful for your test. Then you could trivially expand
your test to try both checksum kinds.

You've resolved the issue with the bare raises, but I'd suggest that
we could make this code a lot cleaner and match what the other tests
are doing but using the assert* functions like they do. I didn't
notice this before since I'm not particularly familiar with the Python
bindings test suite.
Received on 2012-12-11 18:18:42 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.