[Asterisk-code-review] Fix SIP device state notification sporadic failures. (testsuite[master])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: Fix SIP device_state_notification sporadic failures.
......................................................................


Fix SIP device_state_notification sporadic failures.

The SIP device_state_notification test would occasionally fail due to
the SIPp scenario not receiving the expected number of NOTIFY requests.

The problem was due to the timing of the originations from the test plus
the use of the custom device state provider. The test was set to
originate three calls that each would set the device state of a custom
device. Each subsequent call was started as soon as AMI told the test
that the originate was successful. Because of this timing, it is
possible for two calls that set device state to occur very closely
together in parallel.

The custom device state provider works in the following way:
When the device state is set, the new state is written to the astdb and
the device state core is told that the device state has changed for the
custom device state. Now imagine that two threads are changing the
custom device state in close succession. Thread A sets the astdb entry
for the custom device state to be INUSE, and thread B then immediately
sets the device state to be NOT_INUSE. Now both threads alert the device
state core of the changes. Since the astdb now shows the state as
NOT_INUSE, both device state changes appear to change the device state
to NOT_INUSE. The result is that since there are redundant changes, only
one SIP NOTIFY gets sent out, indicating the state as NOT_INUSE. The
expected NOTIFY for the INUSE change does not happen.

This is not an Asterisk problem but a test problem. The fact that
Asterisk may miss an intermediate device state in favor of getting the
current state published is not a bad thing. If the test is going to
rapidly change device state, it cannot expect to always get all
intermediate states.

The fix is to change the test so that it does not start the subsequent
originations until receiving a DeviceStateChange event indicating that
the state has changed on the custom device.

Change-Id: I9722a7c025476ba6625e1a3cf46264bd528b754f
---
M tests/channels/SIP/device_state_notification/originator.py
1 file changed, 8 insertions(+), 5 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/tests/channels/SIP/device_state_notification/originator.py b/tests/channels/SIP/device_state_notification/originator.py
index 4ef8702..0b2b171 100644
--- a/tests/channels/SIP/device_state_notification/originator.py
+++ b/tests/channels/SIP/device_state_notification/originator.py
@@ -32,23 +32,26 @@
     def ami_connect(self, ami):
         LOGGER.info("AMI connected")
         self.ami = ami
+        self.ami.registerEvent('DeviceStateChange', self.device_state_change)
         return
 
-    def success(self, result):
-        LOGGER.info("Originate Successful")
+    def device_state_change(self, ami, event):
+        if event['device'] != 'Custom:Eggs':
+            return
+
+        LOGGER.info("Device state changed to {0}".format(event['state']))
         self.current_destination += 1
         if self.current_destination < len(DESTINATIONS):
             self.originate_call()
-        return result
 
     def originate_call(self):
 
         LOGGER.info("Originating call to %s" %
                 DESTINATIONS[self.current_destination])
 
-        self.ami.originate(channel = 'Local/%s at default' %
+        self.ami.originate(channel='Local/%s at default' %
                 DESTINATIONS[self.current_destination],
-                application = 'Echo').addCallback(self.success)
+                application='Echo')
 
     def scenario_started(self, result):
         def failure(result):

-- 
To view, visit https://gerrit.asterisk.org/2977
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I9722a7c025476ba6625e1a3cf46264bd528b754f
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>