<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/2130/">https://reviewboard.asterisk.org/r/2130/</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 27th, 2012, 6:17 p.m., <b>Matt Jordan</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/2130/diff/1/?file=31391#file31391line102" style="color: black; font-weight: bold; text-decoration: underline;">/asterisk/trunk/lib/python/asterisk/ami.py</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; ">def __event_callback(self, ami, event):</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">102</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="n">k</span><span class="o">.</span><span class="n">lower</span><span class="p">()</span> <span class="ow">not</span> <span class="ow">in</span> <span class="n">event</span><span class="p">:</span></pre></td>
</tr>
<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">103</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">logger</span><span class="o">.</span><span class="n">debug</span><span class="p">(</span><span class="s">"Condition </span><span class="si">%s</span><span class="s"> not in event, returning"</span> <span class="o">%</span> <span class="p">(</span><span class="n">k</span><span class="p">))</span></pre></td>
</tr>
<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">104</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</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;">Actually, it would currently throw an Exception.
If a key is not in an event and we specified that we required it to not match that item, then I don't think we can say anything one way or the other about it. If nothing else, continuing on may give false positives.
Consider matching on this:
Event: Foo
Special: !bar
If we match on Event: Foo and the event doesn't contain Special: at all, we would pass it up to the next level as an event match. But that might not be what we want - what we want are all events that have the Special: field but do not contain bar. If we want all events that merely match Event: Foo but don't contain that header, then we should have just specified matching on Event.</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;">Sorry about this, somehow it decided this comment needed to be off by itself.</pre>
<br />
<p>- Matt</p>
<br />
<p>On September 24th, 2012, 7:37 p.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, Mark Michelson and Terry Wilson.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated Sept. 24, 2012, 7:37 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;">Terry's patch on https://reviewboard.asterisk.org/r/2072/ addresses a fundamental problem in the initial design of ConfBridge, in that it did not adequately define or handle the interactions between Marked, Unmarked, and non-Marked users in a conference. It fixed the various problems reported by users (in numerous ASTERISK issues) by implementing a state machine that moves the conference into various states based on the various types of users entering/leaving the conference.
Because the correct fix to these problems redoes a portion of ConfBridge, tests need to cover the functionality to (a) ensure things continue to work at least as well as they did and (b) that future changes to ConfBridge don't break the interaction behavior between users. This patch provides the following to the Test Suite:
1) A python module (apptest) that allows orchestrating tests inside of a long running application using a configuration driven approach. This uses the same approach as the VoiceMail tests and the existing two ConfBridge tests, but defers the logic of how responses to various events are handled to the YAML file.
2) A test using this that covers the interaction of Marked users in a conference. This covers two scenarios from a ConfBridge test plan (which will be be put up on the wiki and linked to this review later):
Marked users:
Conference should automatically start for marked users and those users should
never be in an inactive conference state.
Actors: Marked User One
Marked User Two w/ Quiet flag
Scenario:
Marked User One enters conference. Marked User One is notified they have
entered conference. Verify no MOH. Marked User Two enters conference.
Verify no announcements played to Marked User Two. Marked User Two leaves
conference. Verify no MOH for Marked User One. Marked User One leaves
conference; verify conference ends.
Scenario:
Marked User Two enters conference. Verify Marked User Two does not
receive notification that they have entered the conference. Verify no MOH.
Marked User One enters conference. Marked User One leaves conference.
Verify no MOH for Marked User Two. Marked User Two leaves conference; verify
conference ends.
</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/lib/python/asterisk/apptest.py <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/sample-yaml/apptest-config.yaml.sample <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/apps/confbridge/confbridge_marked/configs/ast1/confbridge.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/apps/confbridge/confbridge_marked/configs/ast1/extensions.conf <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/tests/apps/confbridge/confbridge_marked/test-config.yaml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/ami.py <span style="color: grey">(3437)</span></li>
<li>/asterisk/trunk/tests/apps/confbridge/tests.yaml <span style="color: grey">(3437)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2130/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>