<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/1364/">https://reviewboard.asterisk.org/r/1364/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 19th, 2011, 9:37 a.m., <b>Paul Belanger</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/1364/diff/2/?file=18487#file18487line7646" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/apps/app_voicemail.c</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; ">static int play_message(struct ast_channel *chan, struct ast_vm_user *vmu, struct vm_state *vms)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">7640</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="n">vms</span><span class="o">-></span><span class="n">curmsg</span> <span class="o">==</span> <span class="n">vms</span><span class="o">-></span><span class="n">lastmsg</span><span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">7646</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></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">7647</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="n">vms</span><span class="o">-></span><span class="n">curmsg</span> <span class="o">==</span> <span class="n">vms</span><span class="o">-></span><span class="n">lastmsg</span><span class="p">)</span> <span class="p">{</span></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;">one line.</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;">Fixed</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 19th, 2011, 9:37 a.m., <b>Paul Belanger</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/1364/diff/2/?file=18487#file18487line10135" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/apps/app_voicemail.c</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; ">static int vm_execmain(struct ast_channel *chan, const char *data)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">10101</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="ew"> </span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">10135</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="ew"> </span></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;">red blob since we are here</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;">Fixed.
I will say that the more test statements I put in, the more of these you'll see - app_voicemail does not follow the coding guidelines well at all. I'd prefer not making a ton of coding guideline fixes to code statements I'm not modifying, as that may be better covered by another effort.
Code statements that I do modify are, of course, fair game :-)</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 19th, 2011, 9:37 a.m., <b>Paul Belanger</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/1364/diff/2/?file=18487#file18487line10239" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/apps/app_voicemail.c</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; ">static int vm_execmain(struct ast_channel *chan, const char *data)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">10198</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">cmd</span><span class="p">)</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">10239</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">cmd</span><span class="p">)</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">10199</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">vms</span><span class="p">.</span><span class="n">repeats</span><span class="o">++</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">10240</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">vms</span><span class="p">.</span><span class="n">repeats</span><span class="o">++</span><span class="p">;</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">10200</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">vms</span><span class="p">.</span><span class="n">repeats</span> <span class="o">></span> <span class="mi">3</span><span class="p">)</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">10241</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">vms</span><span class="p">.</span><span class="n">repeats</span> <span class="o">></span> <span class="mi">3</span><span class="p">)</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">10201</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">cmd</span> <span class="o">=</span> <span class="sc">'t'</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">10242</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">cmd</span> <span class="o">=</span> <span class="sc">'t'</span><span class="p">;</span></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;">brackets since we are here</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;">Fixed, but as you can see from the patch, this turned into another 4 or 5 fixes. We need to be careful not to turn this into a coding guideline patch.</pre>
<br />
<p>- mjordan</p>
<br />
<p>On August 19th, 2011, 10:15 a.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, David Vossel and Paul Belanger.</div>
<div>By mjordan.</div>
<p style="color: grey;"><i>Updated Aug. 19, 2011, 10:15 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;">This review is part 1 of 2.
Previously, when attempting to write tests for complex applications, tests run into two problems:
1. The tests become inherently timing driven. For example, in an application that has a built in playback of a voice file, the test must use Wait(n) or some other sleep mechanism and attempt to wait until the voice prompt is finished before taking action. This can be even more difficult in those applications that have built in menus whose prompts can change based on the state of the application, such as app_voicemail's VoiceMailMain application.
2. Applications whose success is a hangup result can be difficult to verify. While UserEvent can be used in a dialplan to check certain return paths (thus determining whether an application definitely failed), it becomes difficult to say whether or not an application succeeds. For example, Dial has multiple success paths where the caller is hung up on; however, the hang up could also be an error in the dialplan. The only verification mechanism is by inspection.
The TestEvent manager event attempts to solve these two problems. It allows a developer to raise a manager event within an application that notifies the Asterisk Test Suite that something has happened in the application. Currently, this is one of two notifications:
1. A generic change in application state
2. A failure in an assert, indicating an off nominal condition
Both of these statements are macros that evaluate to nothing if the TEST_FRAMEWORK compilation flag is not defined, meaning they should have no impact on non-developer compiled code. While placing the manager events in the code perturbs the units under test, the methods are fairly small, and, in the context of sending audio over a channel for several seconds, are relatively small in terms of timing.
Part 2 of this review (1363) demonstrates a test in the Asterisk Test Suite using this new event. This diff also includes some initial hooks using the TestEvent in app_voicemail's VoiceMailMain application - I expect to add additional hooks as further tests are developed (the current ones check some of the nominal paths and operations)</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>/branches/1.8/configs/manager.conf.sample <span style="color: grey">(331633)</span></li>
<li>/branches/1.8/include/asterisk/manager.h <span style="color: grey">(331633)</span></li>
<li>/branches/1.8/include/asterisk/test.h <span style="color: grey">(331633)</span></li>
<li>/branches/1.8/main/app.c <span style="color: grey">(331633)</span></li>
<li>/branches/1.8/main/file.c <span style="color: grey">(331633)</span></li>
<li>/branches/1.8/main/manager.c <span style="color: grey">(331633)</span></li>
<li>/branches/1.8/main/test.c <span style="color: grey">(331633)</span></li>
<li>/branches/1.8/apps/app_voicemail.c <span style="color: grey">(331633)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1364/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>