> On March 14, 2013, 9:20 a.m., Philip Harvey wrote:
> > 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.
The test directory has experienced a bit of "unconstrained" growth, resulting
in what we have today.
>>To my mind, a lot of the pre-existing tests are either integration or system
>>tests.
I think this is more accident than by design. IIRC, the original test/python
directory was intended for unit tests - exercising the APIs and individual
modules (usually) in isolation. Python was chosen as the driving language
because it allowed the tests to be run with both the proton-j and proton-c
implementations "underneath".
Which is fine, but didn't give us enough test coverage (defects slipped
through), or, specifically in the case of the Messenger tests, required the
full stack (engine + driver), plus separate threads (in order to have two
Messengers talk to each other).
> - I think the apps all relate to soak tests. If so, would you consider
> rearranging the folder structure as shown below?
The proposed layout was somewhat 'forced' by the need to leverage the existing
python-based test harness (the proton-test and common.py files). But that
could change - we could put the python harness in a common place, and break out
the test directories in the manner in which you suggest?
I think perhaps we should move this to the proton mailing list.
> On March 14, 2013, 9:20 a.m., Philip Harvey wrote:
> > /proton/trunk/proton-c/CMakeLists.txt, line 220
> > <https://reviews.apache.org/r/9903/diff/1/?file=270187#file270187line220>
> >
> > 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?
Good point - I was just following a (bad) pattern here. I'll give that a go.
> On March 14, 2013, 9:20 a.m., Philip Harvey wrote:
> > /proton/trunk/tests/apps/messenger/python/msgr-recv.py, line 44
> > <https://reviews.apache.org/r/9903/diff/1/?file=270194#file270194line44>
> >
> > Reformat to use two space indentation?
Gah! Never!!! :)
- Kenneth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9903/#review17868
-----------------------------------------------------------
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
>
>
