[asterisk-dev] [Code Review]: Migration of Asterisk Test Suite to use Python twisted for process management

Matt Jordan reviewboard at asterisk.org
Thu Mar 22 14:05:52 CDT 2012 


> On March 22, 2012, 1:45 p.m., Mark Michelson wrote:
> > From my look through, things look good. With these sorts of changes, it seems like a decent idea would be to get tests that aren't currently derived from TestCase converted so they are. It will take some of the burden of starting and stopping Asterisk from them.
> > 
> > A more python-oriented person might want to take a look through this regarding style, etc.

Yeah, I was trying to limit the scope of this from getting any larger then it already was.  UDPTLv_6 and IAX2 basic-call were done because they were giving me fits, and I had made so many changes to them that I may as well convert them over.

We have tasks to cover tests that are being skipped currently, many of which are the non-TestCase derived tests.  We probably should go ahead and create a task to just do the rest of them as well.


> On March 22, 2012, 1:45 p.m., Mark Michelson wrote:
> > /asterisk/trunk/tests/udptl_v6/run-test, line 126
> > <https://reviewboard.asterisk.org/r/1821/diff/2/?file=26468#file26468line126>
> >
> >     Something tells me this isn't supposed to be here.

You'll find that's in a lot of the test files.  Since I don't use vim for my python development, I assume that comment instructs the vim editor regarding style formatting.  Since it wasn't hurting anything, I left it in when I copied the structure of this test over from the UDTPL test - I'll leave folks who use vim to comment on whether or not its really necessary.


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1821/#review5855
-----------------------------------------------------------


On March 20, 2012, 11:47 a.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1821/
> -----------------------------------------------------------
> 
> (Updated March 20, 2012, 11:47 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> As part of the Lightweight NAT test development (https://reviewboard.asterisk.org/r/1759/), Josh discovered that the Python subprocess module and twisted do not play well together.  At all.  Both twisted and the subprocess attempt to handle SIGCHLD signals, with the result that one or the other (typically twisted, however) fail in some fashion or another.  
> 
> More information on this can be found at the links below:
> 
> http://stackoverflow.com/questions/1948641/twisted-threading-with-subprocess-popen
> http://armyofevilrobots.com/node/422
> http://twistedmatrix.com/trac/ticket/2535
> 
> Note that in the case of the Lightweight NAT tests, the behavior was found without even spawning additional processes; the reactor would block indefinitely when told to listen for UDP packets.
> 
> To solve this, there were a few approaches that we could take:
> 1. Abandon twisted.  This would also imply that we would have to abandon our usage of starpy for FastAGI/AMI integration.  This would not be good, but we could write a non-twisted dependent interface for those two libraries.  However, twisted is a robust library that greatly speeds development of communication interfaces beyond FastAGI/AMI.  For that reason (and a whole host of others), we decided not to go with this.
> 2. Instruct twisted to not use sign handlers by passing installSignalHandlers=False to the reactor.  While this may make subprocess and twisted play nice with each other, it limits the effectiveness of twisted, and could cause issues elsewhere when third party libraries using twisted make the assumption that the reactor will have signal handlers.  This seemed like a workaround, as opposed to a solution.
> 3. Stop using subprocess and use twisted to manage processes.
> 
> We went with option 3.  This has the impact of making a lot of previously blocking operations non-blocking (CLI commands, for example) - which while good, does increase the complexity of those operations.  At the same time, it has the added benefit of removing a number of polling loops and blocking operations that didn't need to be blocking, with the end result that the tests run much, much faster.  (For example: some tests which previously took 15 - 30 seconds now take 2 - 5)
> 
> This patch has the following impacts on test development:
> 1. The Asterisk class now uses non-blocking operations to start/stop itself, and instead returns a deferred object that can be used to be notified when Asterisk has started/stopped.  As a result, tests that derive from TestCase now no longer need to call start_asterisk and stop_asterisk, as the starting/stopping of Asterisk is handled by the TestCase class itself.  Tests that do not use the TestCase object will need to add callbacks to the deferred object to ensure that Asterisk is fully started/stopped.
> 2. The Asterisk class's CLI command interfaces are all non-blocking, and return a deferred object that signals when the command has completed.
> 3. SIPpScenario now returns a deferred when run, which signals when the scenario has ended and its success/failure.
> 4. A new class, SIPpScenarioSequence has been added, which will run a series of SIPpScenario objects in a synchronous fashion.  This eases the burden on people who were using SIPpScenario to run successive tests, as opposed to tests in parallel.
> 5. The utils module has been renamed to TestSuiteUtils, to prevent a name conflict with twisted's utils modules.
> 
> Note that we still have a number of tests that use pjsua and the subprocess module; as we currently have no library wrapping this usage I've deferred (pun intended) that work to another patch.  The Lightweight NAT tests do not use pjsua, and hence are not dependent on that work being finished to be included in the Test Suite.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/udptl_v6/configs/ast1/manager.conf 3104 
>   /asterisk/trunk/tests/udptl_v6/configs/ast2/manager.conf 3104 
>   /asterisk/trunk/tests/udptl_v6/run-test 3104 
>   /asterisk/trunk/tests/mixmonitor/run-test 3104 
>   /asterisk/trunk/tests/mixmonitor_audiohook_inherit/run-test 3104 
>   /asterisk/trunk/tests/fastagi/say-date/run-test 3104 
>   /asterisk/trunk/tests/fastagi/say-datetime/run-test 3104 
>   /asterisk/trunk/tests/fastagi/say-digits/run-test 3104 
>   /asterisk/trunk/tests/fastagi/say-number/run-test 3104 
>   /asterisk/trunk/tests/fastagi/say-phonetic/run-test 3104 
>   /asterisk/trunk/tests/fastagi/say-time/run-test 3104 
>   /asterisk/trunk/tests/feature_attended_transfer/run-test 3104 
>   /asterisk/trunk/tests/feature_blonde_transfer/run-test 3104 
>   /asterisk/trunk/tests/iax2/basic-call/configs/ast1/extensions.conf 3104 
>   /asterisk/trunk/tests/iax2/basic-call/configs/ast1/manager.conf 3104 
>   /asterisk/trunk/tests/iax2/basic-call/configs/ast2/extensions.conf 3104 
>   /asterisk/trunk/tests/iax2/basic-call/configs/ast2/manager.conf 3104 
>   /asterisk/trunk/tests/iax2/basic-call/run-test 3104 
>   /asterisk/trunk/tests/fastagi/say-alpha/run-test 3104 
>   /asterisk/trunk/tests/dynamic-modules/test-config.yaml 3104 
>   /asterisk/trunk/tests/fastagi/get-data/run-test 3104 
>   /asterisk/trunk/tests/fastagi/hangup/run-test 3104 
>   /asterisk/trunk/tests/dynamic-modules/run-test 3104 
>   /asterisk/trunk/tests/chanspy/chanspy_w_mixmonitor/run-test 3104 
>   /asterisk/trunk/tests/chanspy/chanspy_barge/configs/ast1/manager.conf 3104 
>   /asterisk/trunk/tests/chanspy/chanspy_barge/run-test 3104 
>   /asterisk/trunk/tests/channels/SIP/sip_tls_call/configs/ast1/extensions.conf 3104 
>   /asterisk/trunk/tests/channels/SIP/sip_tls_call/configs/ast2/extensions.conf 3104 
>   /asterisk/trunk/tests/channels/SIP/sip_tls_register/run-test 3104 
>   /asterisk/trunk/tests/channels/SIP/use_contact_from_200/run-test 3104 
>   /asterisk/trunk/tests/channels/SIP/sip_register_domain_acl/run-test 3104 
>   /asterisk/trunk/tests/channels/SIP/sip_hold/run-test 3104 
>   /asterisk/trunk/tests/channels/SIP/nat_supertest/test-config.yaml 3104 
>   /asterisk/trunk/tests/channels/SIP/realtime_nosipregs/run-test 3104 
>   /asterisk/trunk/tests/channels/SIP/realtime_sipregs/run-test 3104 
>   /asterisk/trunk/tests/channels/SIP/nat_supertest/sipp/inject.csv 3104 
>   /asterisk/trunk/tests/channels/SIP/nat_supertest/run-test 3104 
>   /asterisk/trunk/tests/channels/SIP/info_dtmf/run-test 3104 
>   /asterisk/trunk/tests/channels/SIP/handle_response_address_incomplete/run-test 3104 
>   /asterisk/trunk/tests/apps/incomplete/sip_incomplete/run-test 3104 
>   /asterisk/trunk/tests/apps/voicemail/func_vmcount/run-test 3104 
>   /asterisk/trunk/lib/python/asterisk/utils.py 3104 
>   /asterisk/trunk/lib/python/asterisk/version.py 3104 
>   /asterisk/trunk/runtests.py 3104 
>   /asterisk/trunk/lib/python/asterisk/sippversion.py 3104 
>   /asterisk/trunk/lib/python/asterisk/sipp.py 3104 
>   /asterisk/trunk/lib/python/asterisk/cdr.py 3104 
>   /asterisk/trunk/lib/python/asterisk/asterisk.py 3104 
>   /asterisk/trunk/lib/python/asterisk/TestConfig.py 3104 
>   /asterisk/trunk/lib/python/asterisk/TestCase.py 3104 
>   /asterisk/trunk/lib/python/asterisk/CDRTestCase.py 3104 
> 
> Diff: https://reviewboard.asterisk.org/r/1821/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120322/2ca930b1/attachment.htm>

More information about the asterisk-dev mailing list