Re: Review Request: Proton-223: Need system-level and soak tests to exercise Proton
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9903/#review18460 --- Ship it! Looks good to me. I highlighted a few TODOs in the code, not because they're showstoppers for me, but to check you're actually intending to commit them :-) /proton/trunk/tests/python/proton_tests/soak.py https://reviews.apache.org/r/9903/#comment38683 I'm not dead-set against committing code containing TODO's but just wanted to check that you're not leaving this in accidentally. /proton/trunk/tests/python/proton_tests/soak.py https://reviews.apache.org/r/9903/#comment38684 I'm not dead-set against committing code containing TODO's but just wanted to check that you're not leaving this in accidentally. /proton/trunk/tests/tools/apps/c/msgr-recv.c https://reviews.apache.org/r/9903/#comment38685 I'm not dead-set against committing code containing TODO's but just wanted to check that you're not leaving this in accidentally. /proton/trunk/tests/tools/apps/c/msgr-recv.c https://reviews.apache.org/r/9903/#comment38686 I'm not dead-set against committing code containing TODO's but just wanted to check that you're not leaving this in accidentally. /proton/trunk/tests/tools/apps/c/msgr-send.c https://reviews.apache.org/r/9903/#comment38687 I'm not dead-set against committing code containing TODO's but just wanted to check that you're not leaving this in accidentally. /proton/trunk/tests/tools/apps/c/msgr-send.c https://reviews.apache.org/r/9903/#comment38688 I'm not dead-set against committing code containing TODO's but just wanted to check that you're not leaving this in accidentally. - Philip Harvey On March 25, 2013, 9:39 p.m., Kenneth Giusti wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9903/ --- (Updated March 25, 2013, 9:39 p.m.) Review request for qpid, Rafael Schloming, Justin Ross, and Philip Harvey. 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/CMakeLists.txt 1457783 /proton/trunk/config.sh 1457783 /proton/trunk/proton-c/CMakeLists.txt 1457783 /proton/trunk/proton-c/src/ssl/openssl.c 1457783 /proton/trunk/tests/python/proton_tests/__init__.py 1457783 /proton/trunk/tests/python/proton_tests/common.py 1457783 /proton/trunk/tests/python/proton_tests/soak.py PRE-CREATION /proton/trunk/tests/python/proton_tests/valgrind.supp PRE-CREATION /proton/trunk/tests/tools/apps/README.txt PRE-CREATION /proton/trunk/tests/tools/apps/c/CMakeLists.txt PRE-CREATION /proton/trunk/tests/tools/apps/c/msgr-common.h PRE-CREATION /proton/trunk/tests/tools/apps/c/msgr-common.c PRE-CREATION /proton/trunk/tests/tools/apps/c/msgr-recv.c PRE-CREATION /proton/trunk/tests/tools/apps/c/msgr-send.c PRE-CREATION /proton/trunk/tests/tools/apps/python/msgr-recv.py PRE-CREATION /proton/trunk/tests/tools/apps/python/msgr-send.py PRE-CREATION /proton/trunk/tests/tools/soak-check 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
Re: Review Request: Proton-223: Need system-level and soak tests to exercise Proton
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9903/ --- (Updated March 25, 2013, 9:39 p.m.) Review request for qpid, Rafael Schloming, Justin Ross, and Philip Harvey. Changes --- Final update: add valgrind support to unit tests. add dedicated soak test script for running long duration the soak tests. 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 (updated) - /proton/trunk/CMakeLists.txt 1457783 /proton/trunk/config.sh 1457783 /proton/trunk/proton-c/CMakeLists.txt 1457783 /proton/trunk/proton-c/src/ssl/openssl.c 1457783 /proton/trunk/tests/python/proton_tests/__init__.py 1457783 /proton/trunk/tests/python/proton_tests/common.py 1457783 /proton/trunk/tests/python/proton_tests/soak.py PRE-CREATION /proton/trunk/tests/python/proton_tests/valgrind.supp PRE-CREATION /proton/trunk/tests/tools/apps/README.txt PRE-CREATION /proton/trunk/tests/tools/apps/c/CMakeLists.txt PRE-CREATION /proton/trunk/tests/tools/apps/c/msgr-common.h PRE-CREATION /proton/trunk/tests/tools/apps/c/msgr-common.c PRE-CREATION /proton/trunk/tests/tools/apps/c/msgr-recv.c PRE-CREATION /proton/trunk/tests/tools/apps/c/msgr-send.c PRE-CREATION /proton/trunk/tests/tools/apps/python/msgr-recv.py PRE-CREATION /proton/trunk/tests/tools/apps/python/msgr-send.py PRE-CREATION /proton/trunk/tests/tools/soak-check 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
Re: Review Request: Proton-223: Need system-level and soak tests to exercise Proton
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9903/ --- (Updated March 21, 2013, 12:36 p.m.) Review request for qpid, Rafael Schloming, Justin Ross, and Philip Harvey. Changes --- Added test for scaling connections and links. 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 (updated) - /proton/trunk/CMakeLists.txt 1457783 /proton/trunk/config.sh 1457783 /proton/trunk/proton-c/CMakeLists.txt 1457783 /proton/trunk/tests/python/proton_tests/__init__.py 1457783 /proton/trunk/tests/python/proton_tests/apps/README.txt PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/c/CMakeLists.txt PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/c/msgr-common.h PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/c/msgr-common.c PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/c/msgr-recv.c PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/c/msgr-send.c PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/python/msgr-recv.py PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/python/msgr-send.py PRE-CREATION /proton/trunk/tests/python/proton_tests/common.py 1457783 /proton/trunk/tests/python/proton_tests/soak.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
Re: Review Request: Proton-223: Need system-level and soak tests to exercise Proton
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9903/ --- (Updated March 18, 2013, 10:11 p.m.) Review request for qpid, Rafael Schloming and Justin Ross. Changes --- Simplified the approach a bit. Rafi recommended parameterizing the tests, and moving them into the proton_tests directory. I've modified the tests so they run very quickly by default - only transferring a small number of messages. Since they are now parameterized, we can manually run them will a much larger message count. With this, they are always run as part of the unit tests (short), but can be optionally run standalone for much longer durations. See the individual tests in soak.py for the parameters. All take at least a 'total_msgs' parameter, which controls the total number of messages the test will transfer. To change a parameter, use the -D (define) option to the proton-test script. Example: ./tests/python/proton-test -Dtotal_msgs=1 -Drecv_count=100 proton_tests.soak.MessengerTests.test_echo_C Eventually I'll add a simple shell script that will run long versions of the soak tests (with valgrind support for the C-based variants). One Issue: 1) could not make these test run under the Jython environment. In reality, I don't think running them under Jython buys us any coverage, since the python is simply used to manage the clients, not actually run any proton code. Correct me if I'm wrong. The problem seems to be a difference with the way Jython does I/O with a subprocess (popen) compared to native python. The apps write to stdout, which the test code reads back. In Jython, the I/O does not become available until the application exits. 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 (updated) - /proton/trunk/CMakeLists.txt 1457783 /proton/trunk/config.sh 1457783 /proton/trunk/proton-c/CMakeLists.txt 1457783 /proton/trunk/tests/python/proton_tests/__init__.py 1457783 /proton/trunk/tests/python/proton_tests/apps/README.txt PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/c/CMakeLists.txt PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/c/msgr-common.h PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/c/msgr-common.c PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/c/msgr-recv.c PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/c/msgr-send.c PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/python/msgr-recv.py PRE-CREATION /proton/trunk/tests/python/proton_tests/apps/python/msgr-send.py PRE-CREATION /proton/trunk/tests/python/proton_tests/common.py 1457783 /proton/trunk/tests/python/proton_tests/soak.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
Re: Review Request: Proton-223: Need system-level and soak tests to exercise Proton
--- 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
Re: Review Request: Proton-223: Need system-level and soak tests to exercise Proton
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.
Re: Review Request: Proton-223: Need system-level and soak tests to exercise Proton
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9903/#review17807 --- /proton/trunk/tests/python/common.py https://reviews.apache.org/r/9903/#comment37754 Note well: this file was moved from test/python/proton_tests up to the test/python directory. For some reason, reviewboard treats it as a new file. /proton/trunk/tests/python/common.py https://reviews.apache.org/r/9903/#comment37755 This method is new, FYI. /proton/trunk/tests/python/proton_tests/common.py https://reviews.apache.org/r/9903/#comment37756 Note: not deleted, just moved up in order to be shared by the two test directories. - Kenneth Giusti 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