[asterisk-dev] [Code Review]: Test device state notifications in chan_sip

[Mark Michelson]

> On Sept. 4, 2012, 9:45 a.m., Matt Jordan wrote:
> > /asterisk/trunk/lib/python/asterisk/TestCase.py, lines 395-399
> > <https://reviewboard.asterisk.org/r/2090/diff/1/?file=31108#file31108line395>
> >
> >     Not that I disagree with this change, but is there a reason why we needed to change this around?

Yes.

The ami_connect() call in the SIPpTestCase class is what starts the SIPp scenarios. When the SIPp scenarios start, our scenario_started_observer (SSO) gets called. In my SSO, I want to originate a call using the AMI reference I saved from my AMI observer. The problem is that my AMI observer has not run because it is set to run after the ami_connect() of the SIPpTestCase. So basically, I try to call None.originate(), which causes python to vomit.

By placing AMI observers to run prior to the test object's ami_connect(), it allows for me to save a reference to the AMI object prior to when the SIPp scenarios start.


> On Sept. 4, 2012, 9:45 a.m., Matt Jordan wrote:
> > /asterisk/trunk/tests/channels/SIP/device_state_notification/test-config.yaml, line 19
> > <https://reviewboard.asterisk.org/r/2090/diff/1/?file=31113#file31113line19>
> >
> >     Python module names should actually be lowercase - so this should be "originator.Originator"
> >     
> >     Yes, we violate this willy nilly all over the test suite (and I'm a prime offender on it), but we probably should be consistent with PEP8 guidelines as much as possible going forward.
> >     
> >     Also: this may be useful in multiple situations.  If so, we may want to add this to the library and make the module a bit more configurable.  We can defer that to a later code change if you'd prefer.

I'll keep PEP8 in mind and will start by making the suggested change here.

I agree that this may be useful in multiple situations. Proof of this would be that I have already written something similar to this for several tests. I don't think it would be too difficult to make this more generic and configurable via YAML.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2090/#review7003
-----------------------------------------------------------


On Aug. 29, 2012, 12:33 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2090/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2012, 12:33 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Due to an issue being reported against Asterisk 11 because of RINGING device state notifications not being sent to SIP phones, I have created a simple test that ensures that device state changes result in notifications being sent to SIP subscribers.
> 
> This test has a single subscriber subscribe to a custom device state. Then the custom device state is changed to RINGING, to INUSE, and then to NOT_INUSE. We ensure that all three changes result in NOTIFYs being sent to the subscriber.
> 
> Note that for this test to pass currently in Asterisk 11 or Asterisk trunk, the patch from ASTERISK-20297 must first be applied. It has not been committed yet.
> 
> 
> This addresses bug ASTERISK-20297.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20297
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/lib/python/asterisk/TestCase.py 3422 
>   /asterisk/trunk/tests/channels/SIP/device_state_notification/Originator.py PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/device_state_notification/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/device_state_notification/configs/ast1/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/device_state_notification/sipp/uac-subscribe-unsubscribe.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/device_state_notification/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/tests.yaml 3422 
> 
> Diff: https://reviewboard.asterisk.org/r/2090/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120904/a05e93f0/attachment.htm>