You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Alan Conway <ac...@redhat.com> on 2013/02/12 18:40:08 UTC
Review Request: PROTON-215: Add tests to verify AMQP type support for python
bindings.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9413/
-----------------------------------------------------------
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
Re: Review Request: PROTON-215: Add tests to verify AMQP 1.0 type support
for all language bindings
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9413/#review16783
-----------------------------------------------------------
Ship it!
Ship It!
- Kenneth Giusti
On Feb. 19, 2013, 5:41 p.m., Alan Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9413/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2013, 5:41 p.m.)
>
>
> Review request for qpid, Kenneth Giusti and Rafael Schloming.
>
>
> Description
> -------
>
> Python and Java tests in place, PHP, Perl and ruby to come.
>
>
> Diffs
> -----
>
> /proton/trunk/proton-c/bindings/java/src/main/java/org/apache/qpid/proton/TestDecoder.java PRE-CREATION
> /proton/trunk/proton-c/bindings/java/src/main/java/org/apache/qpid/proton/codec/jni/JNIData.java 1447222
> /proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/TestDecoder.java PRE-CREATION
> /proton/trunk/tests/interop/message.amqp UNKNOWN
> /proton/trunk/tests/java/org/apache/qpid/proton/InteropTest.java PRE-CREATION
> /proton/trunk/tests/python/interop-generate 1447222
> /proton/trunk/tests/python/proton_tests/interop.py 1447222
>
> Diff: https://reviews.apache.org/r/9413/diff/
>
>
> Testing
> -------
>
> echo "==== python"
> tests/python/proton-test *.interop.* || EXIT=1
> echo "==== proton-jni"
> mvn -P proton-jni -Dtest=InteropTest -DfailIfNoTests=false test || EXIT=1
> echo "==== proton-j"
> mvn -P proton-j -Dtest=InteropTest -DfailIfNoTests=false test || EXIT=1
>
>
> Thanks,
>
> Alan Conway
>
>
Re: Review Request: PROTON-215: Add tests to verify AMQP 1.0 type support
for all language bindings
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9413/
-----------------------------------------------------------
(Updated Feb. 19, 2013, 5:41 p.m.)
Review request for qpid, Kenneth Giusti and Rafael Schloming.
Changes
-------
Java coverage now same as python.
Summary (updated)
-----------------
PROTON-215: Add tests to verify AMQP 1.0 type support for all language bindings
Description (updated)
-------
Python and Java tests in place, PHP, Perl and ruby to come.
Diffs (updated)
-----
/proton/trunk/proton-c/bindings/java/src/main/java/org/apache/qpid/proton/TestDecoder.java PRE-CREATION
/proton/trunk/proton-c/bindings/java/src/main/java/org/apache/qpid/proton/codec/jni/JNIData.java 1447222
/proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/TestDecoder.java PRE-CREATION
/proton/trunk/tests/interop/message.amqp UNKNOWN
/proton/trunk/tests/java/org/apache/qpid/proton/InteropTest.java PRE-CREATION
/proton/trunk/tests/python/interop-generate 1447222
/proton/trunk/tests/python/proton_tests/interop.py 1447222
Diff: https://reviews.apache.org/r/9413/diff/
Testing (updated)
-------
echo "==== python"
tests/python/proton-test *.interop.* || EXIT=1
echo "==== proton-jni"
mvn -P proton-jni -Dtest=InteropTest -DfailIfNoTests=false test || EXIT=1
echo "==== proton-j"
mvn -P proton-j -Dtest=InteropTest -DfailIfNoTests=false test || EXIT=1
Thanks,
Alan Conway
Re: Review Request: PROTON-215: Add type coverage to Java InteropTest.
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9413/
-----------------------------------------------------------
(Updated Feb. 19, 2013, 3:17 p.m.)
Review request for qpid, Kenneth Giusti and Rafael Schloming.
Changes
-------
Added InteropTest.java, tests both JNI and Native. The test uses a subset of the native Decoder interface. On the native build this is actually a Decoder, on JNI it is a wrapper for codec.Data.
Summary (updated)
-----------------
PROTON-215: Add type coverage to Java InteropTest.
Description (updated)
-------
PROTON-215: Add type coverage to Java InteropTest.
Covers all types except:
- described types, described arrays: not yet done
- empty array: throws NPE, possible bug in the decoder.
PROTON-215: Fix bug in array decoding discovered by new tests.
JNIData.getJavaMap() was alwayws returning an empty map.
PROTON-215: Java InteropTest test for primitve types.
Added Java test coverage for just the primitive types, more types coming soon.
PROTON-215: Interop test: make finding AMQP fragments more robust.
Python: find the interop fragments by walking up the directory tree from __file__.
Java: Copy AMQP fragments as resources.
git-svn-id: https://svn.apache.org/repos/asf/qpid/proton/trunk@1446307 13f79535-47bb-0310-9956-ffa450edef68
Diffs (updated)
-----
/proton/trunk/proton-c/bindings/java/src/main/java/org/apache/qpid/proton/TestDecoder.java PRE-CREATION
/proton/trunk/proton-c/bindings/java/src/main/java/org/apache/qpid/proton/codec/jni/JNIData.java 1447222
/proton/trunk/proton-j/proton/src/main/java/org/apache/qpid/proton/TestDecoder.java PRE-CREATION
/proton/trunk/tests/interop/message.amqp UNKNOWN
/proton/trunk/tests/java/org/apache/qpid/proton/InteropTest.java PRE-CREATION
/proton/trunk/tests/python/interop-generate 1447222
/proton/trunk/tests/python/proton_tests/interop.py 1447222
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
Re: Review Request: PROTON-215: Add tests to verify AMQP type support for
python bindings.
Posted by Alan Conway <ac...@redhat.com>.
> 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
>
>
Re: Review Request: PROTON-215: Add tests to verify AMQP type support for
python bindings.
Posted by Rafael Schloming <rh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9413/#review16512
-----------------------------------------------------------
/proton/trunk/proton-c/bindings/python/proton.py
<https://reviews.apache.org/r/9413/#comment35008>
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.
/proton/trunk/tests/python/proton_tests/interop.py
<https://reviews.apache.org/r/9413/#comment35009>
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.
/proton/trunk/tests/python/proton_tests/interop.py
<https://reviews.apache.org/r/9413/#comment35010>
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.
/proton/trunk/tests/python/proton_tests/interop_gen.py
<https://reviews.apache.org/r/9413/#comment35011>
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.
- Rafael Schloming
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
>
>