Jimmy John wrote:
Hello,
I was looking at the codec.py script and decided to write a unt test script
fot it. I wrote 41 tests of which 9 failed. The ones that failed were due to:
(1) No range testing for unsinged integers. e.g. an unsigned octet can take
values from 0 through 255. Hence if we try and encode 256 or -1, the script
should raise an exception.This does not happen with octets, short, long and
long long data types. It fails silently.
(2) In the Field Table data type, the name should have a maximum length of 128. Currently it accepts length greater than this as well. Also, Field names can begin only with letters, $ or #. Currently it accepts a field name with a digit as well.
Attached is a unit test script for codec.py. It is written in the same style
as the existing test harness and can be run independandly or as part of the
run-tests script. Refer the __doc__ string in the script for details of how to
run it.
I'm not sure of what the protocol is for this project. You can review it and
use it/check in etc. If you think these are bugs that need rectification, I can
volunteer to fix them.
Thanks for taking the time to do this. These definitely look like issues
that should be addressed.
Here are my questions/comments on the code:
- The hasattr() check in callFunc, readFunc, etc, seems to obscure
function name typos since they become indistinguishable from assertion
failures. If you drop the hasattr() the getattr() will fail with a clear
error message identifying the problem and it will be reported as an
ERROR rather than a FAILURE by the test harness. This behavior seems
preferable.
- I would drop the use of pack() as this is exactly what the
implementation uses to perform the encoding. A better check would be to
compare directly to the encoded string value just as the field table
tests do. You have this value around already for the decoding tests to
read from, so you could stick it in a constant and reuse it for both
encoding and decoding tests. (See next comment.)
- The last two field table tests check encoding/decoding of the same
value. They both include an encoded representation of the value, and
this string must necessarily be the same for both tests. You could put
this in a constant and share the definition.
- Why do you use dict({...}) in the field table tests rather than just
{...}?
- Why do you use "encode_longstr" instead of "encode_table" for the
field table encoding tests?
- qpid/codec.py (the module itself, not the tests) includes some test
code at the end of the file that should probably be merged into the same
test suite as this. What you've provided is actually a nice compliment
to those tests as the qpid/codec.py test code only checks that
decode(encode(value)) results in the original python value passed in.
Your tests actually check that the encoded format is what is expected
and that error conditions are correctly handled.
If you post this to a JIRA I'd be happy to check it in, although I would
likely revise the few things mentioned above.
--Rafael
