> 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
> 
>

Reply via email to