[asterisk-dev] [Code Review] 2799: Update simple_bridge test for Asterisk 12; update CEL parsing
Matt Jordan
reviewboard at asterisk.org
Tue Sep 10 08:32:59 CDT 2013
> On Sept. 5, 2013, 9:20 p.m., jrose wrote:
> > /asterisk/trunk/lib/python/asterisk/ami.py, line 287
> > <https://reviewboard.asterisk.org/r/2799/diff/1/?file=45241#file45241line287>
> >
> > There should only be one line between function definitions within a class. (flake8)
Fixed
> On Sept. 5, 2013, 9:20 p.m., jrose wrote:
> > /asterisk/trunk/lib/python/asterisk/ami.py, lines 295-297
> > <https://reviewboard.asterisk.org/r/2799/diff/1/?file=45241#file45241line295>
> >
> > Line is too long according to flake8, but 79 character lines are hard to work with, so ehhhhh (flake8)
Reworded
> On Sept. 5, 2013, 9:20 p.m., jrose wrote:
> > /asterisk/trunk/lib/python/asterisk/ami.py, lines 302-304
> > <https://reviewboard.asterisk.org/r/2799/diff/1/?file=45241#file45241line302>
> >
> > class should be followed by two blank lines instead of one (flake8)
Interesting, my pep8 checker doesn't catch that one. Added anyway.
> On Sept. 5, 2013, 9:20 p.m., jrose wrote:
> > /asterisk/trunk/lib/python/asterisk/ami.py, lines 684-691
> > <https://reviewboard.asterisk.org/r/2799/diff/1/?file=45241#file45241line684>
> >
> > More style complaints from flake8:
> > line continuation should indent up to the last parenthesis, so for example...
> >
> > logger.warning("Some stuff %d %d %d" %
> > var1, var2, var3)
> >
> > instead of
> > logger.warning(... %
> > var1, var2, var3
> >
> > Also a handful of lines longer than 79 characters are present, but again... ehhhhh
>
> jrose wrote:
> Only you know... with parenthesis around the vars because it's a tuple.
That's another one my pep8 checker doesn't care about. I'll fix it where I see it.
- Matt
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2799/#review9611
-----------------------------------------------------------
On Aug. 28, 2013, 3:34 p.m., Matt Jordan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2799/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2013, 3:34 p.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Bugs: ASTERISK-22323
> https://issues.asterisk.org/jira/browse/ASTERISK-22323
>
>
> Repository: testsuite
>
>
> Description
> -------
>
> This patch does a few things.
>
> 1) It updates the simple_bridge test for Asterisk 12, such that it handles the correct AMI events, CDR entries, and CEL events.
> 2) It adds a tool (and I emphasize it as a tool to aid in development) that will snoop CEL AMI events and dump them out in YAML. I found this to be helpful when producing the expected CEL events for this test; it may help others as well.
> 3) It updates and simplifies the CEL parsing. More on that below.
>
> -- CEL PARSING --
>
> For a single channel, the CEL events related to that channel should always have a well-defined ordering. That is, for a given test, a channel should always transition through the same states in the same order, e.g.,
> CHAN_START
> APP_START
> BRIDGE_ENTER
> BRIDGE_EXIT
> APP_END
> HANGUP
> CHAN_END
>
> CEL gets tricky, however, because in addition to verifying that the events for a single channel occurred in the correct order, we also care whether or not events across channels occurred in the correct order.
>
> For example, take this test, where Alice calls Bob. The events for each channel may look something like this (simplified greatly):
>
> Alice Bob
> ------- ---------
> CHAN_START CHAN_START
> APP_START ANSWER
> ANSWER BRIDGE_ENTER
> BRIDGE_ENTER BRIDGE_EXIT
> BRIDGE_EXIT HANGUP
> APP_END CHAN_END
> HANGUP
> CHAN_END
>
> Because Alice and Bob will be executing on separate threads, a total ordering of all events is impossible to verify. The notion that such an ordering was ever possible led to the downfall of the original CEL checking. The existing CEL checking handles this, but requires identifying all events and establishing more relationships than is strictly necessary. In reality, what we want to verify between Alice and Bob is closer to the following:
>
> Alice Bob
> ------- ---------
> CHAN_START
> APP_START
> CHAN_START (verify that Bob starts after Alice's application start)
> ANSWER
> ANSWER (verify that Alice answers after Bob answers)
> BRIDGE_ENTER BRIDGE_ENTER (verify that Bob enters the bridge after Alice answers)
> BRIDGE_EXIT BRIDGE_EXIT
> APP_END
> CHAN_END CHAN_END
>
> So what we need are just three orderings between the channels:
> * Bob's CHAN_START must happen after Alice's APP_START
> * Alice's ANSWER must happen after Bob's ANSWER
> * Bob's BRIDGE_ENTER must happen after Alice's ANSWER
>
> The CEL event checking now allows for this by letting an event be tagged with an identifier. Any other event can declare that they have a partial ordering with that event, and state that they must happen before or after that event. If they do not happen before or after that event, the test fails. However, unlike the previous implementation, we don't have to declare identifiers for all events, nor define an ordering for all events.
>
>
> Diffs
> -----
>
> /asterisk/trunk/tests/bridge/simple_bridge/test-config.yaml 4090
> /asterisk/trunk/lib/python/asterisk/cel.py 4090
> /asterisk/trunk/lib/python/asterisk/ami.py 4090
>
> Diff: https://reviewboard.asterisk.org/r/2799/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Matt Jordan
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130910/6089e8a6/attachment-0001.htm>
More information about the asterisk-dev
mailing list