-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9903/#review17868
-----------------------------------------------------------
I generally like what you're doing here.
My only real concern is that the tests folder is starting to look rather
complicated. To some extent this is unavoidable. However, I do have some
suggestions about how to mitigate this:
- To my mind, a lot of the pre-existing tests are either integration or system
tests. Therefore, to avoid confusion in discussions, could we always refer to
the new ones as soak tests?
- We have a mixture of -'s and _'s in file and folder names. Personally I'd
prefer us to use -'s throughout.
- I think the apps all relate to soak tests. If so, would you consider
rearranging the folder structure as shown below?
tests
|
+--java (these are integration tests)
|
+--python (these are integration tests too)
|
+--soak-tests
|
+--apps
| |
| +--(I might collapse the "messenger" layer. It seems unnecessary).
|
+--python
|
+--messenger.py
We might even consider creating an integration-tests folder alongside the
soak-tests folder, but I don't think that's essential right now.
/proton/trunk/proton-c/CMakeLists.txt
<https://reviews.apache.org/r/9903/#comment37834>
I'm no CMake guru but using add_subdirectory to refer to a sibling rather
than child seems strange to me. Would it be possible to move this line to the
top level CMakeLists.txt instead?
/proton/trunk/tests/apps/messenger/python/msgr-recv.py
<https://reviews.apache.org/r/9903/#comment37836>
Reformat to use two space indentation?
/proton/trunk/tests/apps/messenger/python/msgr-send.py
<https://reviews.apache.org/r/9903/#comment37835>
All the other files use two-space indentation but this one uses four spaces.
- Philip Harvey
On March 13, 2013, 5:19 p.m., Kenneth Giusti wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9903/
> -----------------------------------------------------------
>
> (Updated March 13, 2013, 5:19 p.m.)
>
>
> Review request for qpid, Rafael Schloming and Justin Ross.
>
>
> Description
> -------
>
> This patch puts the infrastructure in place for system and soak tests as
> described in proton-223.
>
> In summary:
>
> o) it adds a set of simple messenger-based applications for sending a
> receiving messages; implementations done in python and C (for now).
> o) creates a set of python test scripts for driving these tests
>
> Right now I've got a small set of tests that generate traffic between the
> applications and verify that no messages are dropped. These are run as part
> of "ctest", or can be run directly using the python test tool:
>
> ./test/python/proton-test -m system_tests
>
>
> Todo:
>
> 1) I need a Java-based version of the Messenger apps. Other languages are
> desired too, but we need Java for coverage of that implementation.
> 2) Enhance the receivers to explicitly accept messages based on window size.
> 3) More tests - connection scale, link scale, a test that covers proton-131,
> long running traffic tests, etc
> 4) Benchmark tests and related support for gathering throughput and latency.
> 5) Valgrind coverage exists for the C application, but fails for any cross
> language tests. A small set of C-only tests would be useful for valgrind
> tests.
> 6) anything else???
>
>
> This addresses bugs proton-131 and proton-223.
> https://issues.apache.org/jira/browse/proton-131
> https://issues.apache.org/jira/browse/proton-223
>
>
> Diffs
> -----
>
> /proton/trunk/config.sh 1455926
> /proton/trunk/proton-c/CMakeLists.txt 1455926
> /proton/trunk/tests/apps/README.txt PRE-CREATION
> /proton/trunk/tests/apps/messenger/c/CMakeLists.txt PRE-CREATION
> /proton/trunk/tests/apps/messenger/c/msgr-common.h PRE-CREATION
> /proton/trunk/tests/apps/messenger/c/msgr-common.c PRE-CREATION
> /proton/trunk/tests/apps/messenger/c/msgr-recv.c PRE-CREATION
> /proton/trunk/tests/apps/messenger/c/msgr-send.c PRE-CREATION
> /proton/trunk/tests/apps/messenger/python/msgr-recv.py PRE-CREATION
> /proton/trunk/tests/apps/messenger/python/msgr-send.py PRE-CREATION
> /proton/trunk/tests/python/common.py PRE-CREATION
> /proton/trunk/tests/python/proton_tests/common.py 1455926
> /proton/trunk/tests/python/system_tests/__init__.py PRE-CREATION
> /proton/trunk/tests/python/system_tests/messenger.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9903/diff/
>
>
> Testing
> -------
>
> The short tests have been added to ctest, only tested on Linux (fedora 17)
> thus far.
>
>
> Thanks,
>
> Kenneth Giusti
>
>
