qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan Conway" <acon...@redhat.com>
Subject Re: Review Request: PROTON-215: Add tests to verify AMQP type support for python bindings.
Date Fri, 15 Feb 2013 14:42:22 GMT


> On Feb. 13, 2013, 3:08 p.m., Rafael Schloming wrote:
> > /proton/trunk/proton-c/bindings/python/proton.py, line 967
> > <https://reviews.apache.org/r/9413/diff/1/?file=257860#file257860line967>
> >
> >     I don't recall offhand if this notation works on python 2.4. If you haven't
already, we should probably check just to be sure.

According to docs it does.


> On Feb. 13, 2013, 3:08 p.m., Rafael Schloming wrote:
> > /proton/trunk/tests/python/proton_tests/interop.py, line 23
> > <https://reviews.apache.org/r/9413/diff/1/?file=257862#file257862line23>
> >
> >     I'm thinking we should actually check the data files into svn and load them
from disk rather than generate them on the fly. I think this would be good for two reasons
(1) the unit tests in other languages wouldn't need to depend on python, and (2) we would
also catch compensating bugs, e.g. if I generate the test data on the fly all the time then
I could introduce an encode bug along with a compensating decode bug and the tests wouldn't
catch it, whereas if we check the files in, then we can verify that the current decode will
work properly against historic versions of the encode as well.

Agreed, I was debating that. Will switch to checked-in.


> On Feb. 13, 2013, 3:08 p.m., Rafael Schloming wrote:
> > /proton/trunk/tests/python/proton_tests/interop_gen.py, line 53
> > <https://reviews.apache.org/r/9413/diff/1/?file=257863#file257863line53>
> >
> >     I'm wondering if we'll want to define a format such that we can add multiple
values to check edge cases, e.g. binary with nulls in it, or a string with unicode characters,
empty binary, string, and symbol values, etc.
> >     
> >     I'm not sure offhand what the best way to deal with that would be. I guess in
some ways we'd need to update all the unit tests anyways in order to add to an extra assertion,
so there might not be all that much point to a generic format.
> >     
> >     Either way I'd suggest starting out with as many edge cases as we can think
of so when we port the python unit tests you have to other languages we get the additional
coverage. Edgecases are a pretty common place where subtle mapping issues creep in. It's easy
to confuse binary, string, and symbol and map them all into the same thing if you don't have
sample values that exercise their differences.

Agreed we need more edge cases. We can generate strings with nulls etc. with what we have,
the only thing we can't generate is ill-formed AMQP. We could manually put together some of
those cases. 


> On Feb. 13, 2013, 3:08 p.m., Rafael Schloming wrote:
> > /proton/trunk/tests/python/proton_tests/interop.py, line 33
> > <https://reviews.apache.org/r/9413/diff/1/?file=257862#file257862line33>
> >
> >     I'm wondering if we shouldn't just add methods to the Message class to directly
encode/decode to/from binary data. This seems like it might be convenient here and its also
plausible that end users would find it useful. It would also let us check interop on a few
small aspects of the message encode/decode that might not be covered by just checking through
the data interface here.

Will add it.


- Alan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9413/#review16512
-----------------------------------------------------------


On Feb. 12, 2013, 5:40 p.m., Alan Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9413/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2013, 5:40 p.m.)
> 
> 
> Review request for qpid, Kenneth Giusti and Rafael Schloming.
> 
> 
> Description
> -------
> 
> PROTON-215: Add tests to verify AMQP type support for python bindings.
> 
> The test consists of
> - a generator program that generates AMQP fragments for all AMQP types
> - a set of unit tests to decode, verify and re-encode the fragments.
> 
> This will be extended intended to cover all the proton language bindings.  All bindings
> will re-use the generator, unit tests will be added for each language.
> 
> 
> Diffs
> -----
> 
>   /proton/trunk/proton-c/bindings/python/proton.py 1443924 
>   /proton/trunk/tests/python/proton_tests/__init__.py 1443924 
>   /proton/trunk/tests/python/proton_tests/interop.py PRE-CREATION 
>   /proton/trunk/tests/python/proton_tests/interop_gen.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/9413/diff/
> 
> 
> Testing
> -------
> 
> described_array test fails, skipping. I believe the tests have found their first bug
:)
> 
> 
> Thanks,
> 
> Alan Conway
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message