<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1821/">https://reviewboard.asterisk.org/r/1821/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 22nd, 2012, 1:45 p.m., <b>Mark Michelson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/1821/diff/2/?file=26468#file26468line126" style="color: black; font-weight: bold; text-decoration: underline;">/asterisk/trunk/tests/udptl_v6/run-test</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">70</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "># vim:sw=4:ts=4:expandtab:textwidth=79</pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Something tells me this isn't supposed to be here.</pre>
</blockquote>
<p>On March 22nd, 2012, 2:05 p.m., <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It's not strictly necessary ... but it sets up some basics for PEP8 style. Everyone working on Python should just add Python settings into their vimrc file, and then it's not necessary at all.
Python related stuff from my ~/.vimrc:
" Syntax Highlighting stuff.
let python_highlight_all=1
syntax on
" Python specific formatting settings
au BufRead,BufNewFile *.py,*.pyc set sw=4 ts=4 expandtab textwidth=79
au BufNewFile *.py,*.pyw set fileformat=unix
" Highlight any leading tabs in python code
highlight BadWhitespace ctermbg=red guibg=red
au BufRead,BufNewFile *.py,*.pyw match BadWhitespace /^\t\+/
Also relevant: http://svn.python.org/projects/python/trunk/Misc/Vim/vimrc</pre>
<br />
<p>- Russell</p>
<br />
<p>On March 23rd, 2012, 8:42 a.m., Matt Jordan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated March 23, 2012, 8:42 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-19409">ASTERISK-19409</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/asterisk/trunk/tests/mixmonitor_audiohook_inherit/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/udptl_v6/configs/ast1/manager.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/udptl_v6/configs/ast2/manager.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/udptl_v6/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/mixmonitor/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/feature_blonde_transfer/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/iax2/basic-call/configs/ast1/extensions.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/iax2/basic-call/configs/ast1/manager.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/iax2/basic-call/configs/ast2/extensions.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/iax2/basic-call/configs/ast2/manager.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/iax2/basic-call/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-number/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-phonetic/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-time/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/feature_attended_transfer/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-digits/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-datetime/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-date/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/dynamic-modules/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/dynamic-modules/test-config.yaml <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/get-data/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/hangup/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/fastagi/say-alpha/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/chanspy/chanspy_w_mixmonitor/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/chanspy/chanspy_barge/configs/ast1/manager.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/chanspy/chanspy_barge/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/use_contact_from_200/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_tls_call/configs/ast2/extensions.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_tls_register/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_tls_call/configs/ast1/extensions.conf <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_register_domain_acl/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/cdr.py <span style="color: grey">(3111)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/sipp.py <span style="color: grey">(3111)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/sippversion.py <span style="color: grey">(3111)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/utils.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/version.py <span style="color: grey">(3111)</span></li>
<li>/asterisk/trunk/runtests.py <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/apps/incomplete/sip_incomplete/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/check_voicemail_forward_with_prepend/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/check_voicemail_new_user_hangup/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/func_vmcount/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/handle_response_address_incomplete/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/info_dtmf/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/sipp/inject.csv <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/nat_supertest/test-config.yaml <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/realtime_nosipregs/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/realtime_sipregs/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/tests/channels/SIP/sip_hold/run-test <span style="color: grey">(3104)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/TestConfig.py <span style="color: grey">(3111)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/asterisk.py <span style="color: grey">(3111)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/TestCase.py <span style="color: grey">(3111)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/CDRTestCase.py <span style="color: grey">(3111)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1821/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>