[Asterisk-code-review] Testsuite: Refactored Batched RLS Test Driver for Better Log... (testsuite[master])

Joshua Colp asteriskteam at digium.com
Wed Nov 11 15:52:28 CST 2015


Joshua Colp has submitted this change and it was merged.

Change subject: Testsuite: Refactored Batched RLS Test Driver for Better Logging
......................................................................


Testsuite: Refactored Batched RLS Test Driver for Better Logging

While this does not actually correct the issue with the test failure (which
incidentally turns out is not localized to this test), this patch modifies the
the test driver such that the test output produces log messages that help to
elimanate it as the suspect in the test failure. Also, this patch includes
fixes for pylint errors (e.g. missing docstrings).

This is part one (1) of an n-patch series of refactorings to help determine
the root cause of the test failure and correct pylint issues.

ASTERISK-25430
Reported By: Ashley Sanders

Change-Id: I15c005ec6a43919e5fe533c8f2dc8fb3d5c41e64
---
M tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/batched/basic/driver.py
M tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/batched/basic/test-config.yaml
2 files changed, 266 insertions(+), 64 deletions(-)

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



diff --git a/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/batched/basic/driver.py b/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/batched/basic/driver.py
index b961a85..9b46149 100755
--- a/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/batched/basic/driver.py
+++ b/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/batched/basic/driver.py
@@ -1,4 +1,11 @@
 #!/usr/bin/env python
+"""
+Copyright (C) 2015, Digium, Inc.
+Mark Michelson <mmichelson at digium.com>
+
+This program is free software, distributed under the terms of
+the GNU General Public License Version 2.
+"""
 
 import time
 import logging
@@ -24,30 +31,218 @@
 # This state is entered after Bob's device state NOTIFY has been received.
 BOB_STATE = 5
 
-# This state is entered after SUBSCRIBE-NOTIFY exchange to terminate
-# subscription has occurred.
-TERMINATED = 6
+def named_constant_to_text(state):
+    """Converts a named state constant to textual representation.
+
+    Keyword Arguments:
+    state                      -- The named state constant.
+
+    Returns:
+    A string representing the named constant.
+    """
+
+    if state == UNESTABLISHED:
+        return 'UNESTABLISHED'
+    elif state == ESTABLISHED:
+        return 'ESTABLISHED'
+    elif state == ALICE_STATE:
+        return 'ALICE_STATE'
+    elif state == REFRESHED:
+        return 'REFRESHED'
+    elif state == BOB_STATE:
+        return 'BOB_STATE'
+    return 'UNKNOWN'
+
+def build_users_list():
+    """Builds the list of users.
+
+    Returns:
+    A list of TestUser objects.
+    """
+
+    users = list()
+
+    for info in [('Alice', ESTABLISHED), ('Bob', REFRESHED)]:
+        name = info[0]
+        user = TestUser(name,
+                        info[1],
+                        globals()[name.upper() + '_STATE'])
+        users.append(user)
+    return users
+
+
+class TestUser(object):
+    """A test user.
+
+    This class is responsbile for keeping track of the start/end states,
+    transition time interval, and current state for a SipPScenario actor.
+    """
+
+    def __init__(self, name, start_state, end_state):
+        """Constructor.
+
+        Keyword Arguments:
+        name                   -- The name for this TestUser instance
+        start_state            -- The state to use when this user is activated.
+        end_state              -- The state to use when this user is
+                                  deactivated.
+        """
+
+        self.name = name
+        self.state = UNESTABLISHED
+        self.__start_state = start_state
+        self.__end_state = end_state
+        self.__start_time = 0.0
+        self.__end_time = 0.0
+
+    def activate(self, ami):
+        """Activates this TestUser.
+
+        This will update the state of the user to the start_state and send an
+        AMI message which triggers the SipPScenario to transition its state."""
+
+        message = "Activating user: {0}."
+        LOGGER.debug(message.format(self.name))
+
+        self.state = self.__start_state
+        self.__start_time = time.time()
+
+        ami_message = {
+            'Action': 'SetVar',
+            'Variable': 'DEVICE_STATE(Custom:{0})'.format(self.name.lower()),
+            'Value': 'InUse'
+        }
+        ami.sendMessage(ami_message)
+
+    def deactivate(self):
+        """Deactivates this TestUser."""
+
+        message = "Deactivating user: {0}."
+        LOGGER.debug(message.format(self.name))
+
+        self.state = self.__end_state
+        self.__end_time = time.time()
+
+    def check_elapsed_time(self):
+        """Checks the elapsed between acitvation and deactivation."""
+
+        message = "Checking time interval for {0}."
+        LOGGER.debug(message.format(self.name))
+
+        message = "Time interval for {0}: {1}. Check {2}."
+        interval = self.__end_time - self.__start_time
+        if interval < 5.0:
+            LOGGER.error(message.format(self.name, interval, "failed"))
+            return False
+
+        LOGGER.debug(message.format(self.name, interval, "passed"))
+        return True
 
 
 class TestDriver(object):
+    """The driver for the test.
+
+    This class is responsbile for initiating the state changes for the
+    SipPScenario actors.
+    """
+
     def __init__(self, module_config, test_object):
+        """Constructor.
+
+        Keyword Arguments:
+        module_config          -- The YAML configuration for this test.
+        test_object            -- The TestCaseModule instance for this test.
+        """
+
         self.ami = None
-        self.state = UNESTABLISHED
         self.test_object = test_object
-        self.alice_time = 0.0
-        self.bob_time = 0.0
-        test_object.register_ami_observer(self.ami_connect)
+
+        self.__current_user = None
+        self.__users = build_users_list()
+        self.__iterator = iter(self.__users)
+
+        test_object.register_ami_observer(self.on_ami_connect)
+
+    def check_state(self, valid_states):
+        """Checks the state of the test.
+
+        Keyword Arguments:
+        valid_states           -- A list of states that are valid.
+
+        Returns:
+        True if the test state matches at least one of the valid states.
+        Otherwise, returns False.
+        """
+
+        current_state = self.get_state()
+        message = "Checking that the test state: {0} is valid."
+        LOGGER.debug(message.format(current_state))
+
+        for state in valid_states:
+            if state == current_state:
+                LOGGER.debug("Test state is valid.")
+                return True
+
+        states = \
+            ', or '.join([named_constant_to_text(x) for x in valid_states])
+        LOGGER.error("Unexpected test state. Expected: {0}.".format(states))
+        return False
 
     def fail_test(self):
+        """Fails the test."""
+
+        LOGGER.error("Failing the test.")
         self.test_object.set_passed(False)
         self.test_object.stop_reactor()
 
-    def ami_connect(self, ami):
-        self.ami = ami
-        self.ami.registerEvent('TestEvent', self.on_test_event)
+    def get_state(self):
+        """Gets the test state.
 
-    def on_test_event(self, ami, event):
+        Returns:
+        If the current user is None, this will return UNESTABLISHED. Otherwise,
+        the state of the current user is returned.
+        """
+
+        if self.__current_user is None:
+            return UNESTABLISHED
+
+        return self.__current_user.state
+
+    def log_state(self, caller):
+        """Logs the state of the test for a given caller.
+
+        Keyword Arguments:
+        caller                 -- The caller requesting the state to be logged.
+        """
+
+        message = "In {0}. self.state={1}."
+        LOGGER.debug(message.format(caller.__name__,
+                                    named_constant_to_text(self.get_state())))
+
+    def on_ami_connect(self, ami):
+        """Handles the AMI connect event.
+
+        Keyword Arguments:
+        ami                    -- The AMI instance that raised this event.
+        """
+
+        self.ami = ami
+        self.ami.registerEvent('TestEvent', self.on_ami_test_event)
+
+    def on_ami_test_event(self, ami, event):
+        """Handles an AMI TestEvent.
+
+        Routes the test data according to the state of the AMI event.
+
+        Keyword Arguments:
+        sender                 -- The ami instance that raised the event.
+        event                  -- The event payload.
+        """
+
         state = event['state']
+        message = "In self.on_test_event. event[state]={0}."
+        LOGGER.debug(message.format(state))
+
         if state == 'SUBSCRIPTION_ESTABLISHED':
             self.on_subscription_established()
         elif state == 'SUBSCRIPTION_REFRESHED':
@@ -55,70 +250,74 @@
         elif state == 'SUBSCRIPTION_TERMINATED':
             self.on_subscription_terminated()
         elif state == 'SUBSCRIPTION_STATE_CHANGED':
-            self.on_subscription_state_change()
+            self.on_subscription_state_changed()
 
     def on_subscription_established(self):
-        if self.state != UNESTABLISHED:
-            LOGGER.error("Unexpected state change from {0}. Expected state "
-                         "{1}".format(self.state, UNESTABLISHED))
+        """Handles an AMI 'SUBSCRIPTION_ESTABLISHED' TestEvent.
+
+        Verifies the current state matches the expected state and transitions
+        the TestUser.
+        """
+
+        self.log_state(self.on_subscription_established)
+        if not self.check_state([UNESTABLISHED]):
             self.fail_test()
-
-        LOGGER.debug("State change to {0}".format(ESTABLISHED))
-        self.state = ESTABLISHED
-        self.alice_time = time.time()
-        message = {
-            'Action': 'SetVar',
-            'Variable': 'DEVICE_STATE(Custom:alice)',
-            'Value': 'InUse'
-        }
-        self.ami.sendMessage(message)
-
-    def on_subscription_state_change(self):
-        if self.state != ESTABLISHED and self.state != REFRESHED:
-            LOGGER.error("Unexpected state change from {0}. Expected state "
-                         "{1} or {2}".format(self.state, ESTABLISHED,
-                                             REFRESHED))
-            self.fail_test()
-
-        if self.state == ESTABLISHED:
-            interval = time.time() - self.alice_time
-            if interval < 5.0:
-                LOGGER.error("Interval {0} too brief".format(interval))
-                self.fail_test()
-            LOGGER.debug("State change to {0}".format(ALICE_STATE))
-            self.state = ALICE_STATE
-        if self.state == REFRESHED:
-            interval = time.time() - self.bob_time
-            if time.time() - self.bob_time < 5.0:
-                LOGGER.error("Interval {0} too brief".format(interval))
-                self.fail_test()
-            LOGGER.debug("State change to {0}".format(BOB_STATE))
-            self.state = BOB_STATE
+        self.transition_user()
 
     def on_subscription_refreshed(self):
-        if self.state != ALICE_STATE:
-            LOGGER.error("Unexpected state change from {0}. Expected state "
-                         "{1}".format(self.state, ALICE_STATE))
+        """Handles an AMI 'SUBSCRIPTION_REFRESHED' TestEvent.
+
+        Verifies the current state matches the expected state and transitions
+        the TestUser.
+        """
+
+        self.log_state(self.on_subscription_refreshed)
+        if not self.check_state([ALICE_STATE]):
+            self.fail_test()
+        self.transition_user()
+
+    def on_subscription_state_changed(self):
+        """Handles an AMI 'SUBSCRIPTION_STATE_CHANGED' TestEvent.
+
+        Deactivates the current user and verifies that its time inteval
+        is greater than the tolerance level (5 seconds).
+        """
+
+        self.log_state(self.on_subscription_state_changed)
+        if not self.check_state([ESTABLISHED, REFRESHED]):
             self.fail_test()
 
-        LOGGER.debug("State change to {0}".format(REFRESHED))
-        self.state = REFRESHED
-        self.bob_time = time.time()
-        message = {
-            'Action': 'SetVar',
-            'Variable': 'DEVICE_STATE(Custom:bob)',
-            'Value': 'InUse'
-        }
-        self.ami.sendMessage(message)
+        self.__current_user.deactivate()
+        if not self.__current_user.check_elapsed_time():
+            self.fail_test()
 
     def on_subscription_terminated(self):
-        if self.state != BOB_STATE:
-            LOGGER.error("Unexpected state change from {0}. Expected state "
-                         "{1}".format(self.state, BOB_STATE))
+        """Handles an AMI SUBSCRIPTION_TERMINATED TestEvent.
+
+        Verifies that the final state matches the expected state (BOB_STATE)
+        and marks the test as passed or failed."""
+
+        self.log_state(self.on_subscription_terminated)
+        if not self.check_state([BOB_STATE]):
             self.fail_test()
 
-        LOGGER.debug("State change to {0}".format(TERMINATED))
-        self.state = TERMINATED
         # If we've made it here, then the test has passed!
         self.test_object.set_passed(True)
         self.test_object.stop_reactor()
+
+    def transition_user(self):
+        """ Transitions the current user to the next user in the list.
+
+        This will set the value of current_user to the next available user in
+        the list, or None, if we are at the end of the list. If we are at the
+        end of the list, nothing else is done. Otherwise, the current_user is
+        activated.
+        """
+
+        try:
+            self.__current_user = self.__iterator.next()
+        except StopIteration:
+            self.__current_user = None
+            return
+
+        self.__current_user.activate(self.ami)
diff --git a/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/batched/basic/test-config.yaml b/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/batched/basic/test-config.yaml
index e10c845..dc55844 100644
--- a/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/batched/basic/test-config.yaml
+++ b/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/batched/basic/test-config.yaml
@@ -32,6 +32,8 @@
     add-to-search-path:
         -
             'tests/channels/pjsip/subscriptions/rls'
+    reactor-timeout: 25
+    fail-on-any: 'True'
     test-object:
         config-section: 'test-case-config'
         typename: 'sipp.SIPpTestCase'
@@ -49,6 +51,7 @@
                 - { 'key-args': {'scenario': 'subscribe.xml', '-i': '127.0.0.1', '-p': '5061', '-s': 'pres_list'} }
 
 test-config:
+    stop_test_notifys: False
     list_name: 'pres_list'
     resources:
         -

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I15c005ec6a43919e5fe533c8f2dc8fb3d5c41e64
Gerrit-PatchSet: 4
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-code-review mailing list