<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/1447/">https://reviewboard.asterisk.org/r/1447/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 20th, 2011, 12:27 p.m., <b>Matthew Nicholson</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/1447/diff/1/?file=20790#file20790line344" style="color: black; font-weight: bold; text-decoration: underline;">/asterisk/trunk/README.txt</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">344</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> name: 'asterisk.ThreadTestCondition.ThreadPostTestCondition'</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;">Is there a way to use some sort of short hand for these extremely long condition names?</pre>
</blockquote>
<p>On September 20th, 2011, 12:38 p.m., <b>mjordan</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;">Nope.
We need the python module name (the thing after the first .) and the class name for the introspection to properly lookup the object to dynamically create. The 'asterisk' is in there because our python system library path doesn't append that folder to it, and changing that would affect a lot of import statements throughout the code.</pre>
</blockquote>
<p>On September 20th, 2011, 12:44 p.m., <b>Paul Belanger</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;">I would agree. I like where we are going, however we are starting to make it complex for writing a test config file. I would must rather see:
reports:
Threads: yes
SIPDialogs: yes
Ignore: 'netconsole'
...
Then the test suite is responsible for building up the conditions. At this point, I'm not sure why or how a Pre / Post check condition for Threads could be different or changed.</pre>
</blockquote>
<p>On September 20th, 2011, 12:48 p.m., <b>mjordan</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;">Well, we either specify the objects to create in the configuration and do it dynamically, or we hard code them in TestCase. I had them hard coded originally, but that made adding new ones rather tough on the developer - they had to find the correct place in the base class and add all the right callbacks and hooks to have them added correctly and not break things.
I can go with that approach instead of that's what people would like.</pre>
</blockquote>
<p>On September 20th, 2011, 12:52 p.m., <b>mjordan</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;">Whoops, wrong class. They'd be hard coded in TestConditions.</pre>
</blockquote>
<p>On September 20th, 2011, 1 p.m., <b>Paul Belanger</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;">:p If you are getting confused, we are in trouble :D Kidding.
I don't know the best approach to take, will let you make the call, but if we can make the configuration file more simple I think it will be better. We should just be passing a flag to enable / disable the thread tests, and the testsuite takes care of the rest. </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;">If you know that what you're doing is specifying the name of the object to create, I don't think what we have is very difficult or cumbersome. Is it longer then just a 'yes' or 'no'? Sure. But it also gives us the capability to not have to go digging into base classes to find the right place - and the right method calls - to register a new pre- / post-test condition. That's pretty handy - particularly since I'm not going to be the only person writing them (I hope).
As far as having to do test configuration files, this patch makes it less of a burden by defining the pessimistic mode checks in a top level file, so that they are only defined once. As a test writer, you should test your test case in pessimistic mode to determine - with the defaults - what checks pass and fail. If you have checks that fail, and they are not issues with Asterisk, you can override the default behavior in your test-config.yaml file. In that case, you will need to redefine the pre / post-test conditions that you need to override - but Ctrl+C / Ctrl+V works just as fast for 30 characters as it does for 5 (and if you're retyping everything and not just the stuff you need, well, then, I can't help you).</pre>
<br />
<p>- mjordan</p>
<br />
<p>On September 19th, 2011, 2:22 p.m., mjordan 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 and Paul Belanger.</div>
<div>By mjordan.</div>
<p style="color: grey;"><i>Updated Sept. 19, 2011, 2:22 p.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;">This patch adds the ability for global 'modes' to be defined for the Asterisk test-suite. Those settings in a mode can then be applied either at the base level in the runtests.py script, or by any test that inherits from TestClass.py. As the global settings file makes use of the TestConfig class, all settings in the TestConfig class can theoretically be applied on a global scale - although since each test checks its dependencies independently, only the Test Conditions are currently useful.
One additional settings was added - exclude-tests. This lets the global settings file explicitly call out tests (or subsets of tests) to not run without having to modify the tests.yaml file, and acts as a counterpart to the --test command line flag.
The only assumption made by this patch is that the global settings file is named 'test-config.yaml' and is located at the current run directory (the location of the runtests.py script).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">Tested fast config mode, standard config mode, and pessimistic config mode. Each worked as intended:
* Fast config mode successfully excluded those tests with a reactor_timeout greater than one minute
* Standard config mode applied no global settings
* Pessimistic config mode applied the Test Conditions to all tests inheriting from TestCase. Those tests that further defined test specific overrides applied those settings over the global conditions.</pre>
</td>
</tr>
</table>
<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/README.txt <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/TestCase.py <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/TestConfig.py <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/runtests.py <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/authenticate_extensions/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/authenticate_invalid_mailbox/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/authenticate_invalid_password/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/authenticate_nominal/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/check_voicemail_callback/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/check_voicemail_delete/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/check_voicemail_dialout/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/check_voicemail_envelope/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/check_voicemail_new_user/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/check_voicemail_nominal/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/check_voicemail_reply/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/func_vmcount/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/leave_voicemail_external_notification/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/leave_voicemail_forwarding/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/leave_voicemail_forwarding_auto_urgent/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/test-config.yaml <span style="color: grey">(2346)</span></li>
<li>/asterisk/trunk/tests/apps/voicemail/leave_voicemail_priority/test-config.yaml <span style="color: grey">(2346)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1447/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>