[Asterisk-code-review] Testsuite: fix several sporadically failing tests (testsuite[certified/18.9])
Friendly Automation
asteriskteam at digium.com
Tue Apr 26 13:25:54 CDT 2022
Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/testsuite/+/18433 )
Change subject: Testsuite: fix several sporadically failing tests
......................................................................
Testsuite: fix several sporadically failing tests
This patches fixes problems across several tests:
A few of the 'pjsip/qualify' tests were failing due to a race between
the SIPp scenario, and the AMIEventModule both trying to stop the test.
This patch fixes the issue by ensuring the test is stopped only after
the expected AMI event is received.
The 'rest_api/applications/subscribe-endpoint/nominal/tech' test was
also failing due to a race condition between the order in which ARI
events were received. Under certain circumstances the event that
initiates the end of the test could be received before other required
events. This test was refactored to make it so 'stop_test' action does
does not trigger until all expected events are received.
The 'pjsip/subscriptions/presence/presencestate_repeat' test was failing
due to a race between initializing presence at startup and the SIPp
scenario subscribing. Depending on timing it was possible for the scenario
to start, and try to subscribe before the presence was initialized. When
this happened the test would fail. The solution was to guarantee the
scenario does not start until Asterisk has time to fully initialze its
state for the test. As such this patch adds a new 'SIPpActionModule' that
allows SIPp scenarios to now be triggered within an 'EventAction' setup.
Lastly, despite previous efforts to solve the problem some tests still
fail due to a 'port in use' error. Albeit, previous solutions have
lessened occurrences. The problem was ports in the testsuite were being
reserved based on the test's SIP listening port type, i.e UDP vs TCP.
However, the ports needing to be reserved are the media ports which are
all UDP based. This means TCP based tests were reserving TCP based media
ports, which can overlap. But the reservation of such ports essentially
have no meaning since SIPp attempts to bind using UDP. This patch ensures
reserving of all media ports now takes place using UDP only.
Change-Id: I5d8bb49ba8c8238ba76fd2545ff9be2ff0154399
---
M lib/python/asterisk/matcher_listener.py
M lib/python/asterisk/pluggable_modules.py
M lib/python/asterisk/self_test/test_utils_socket.py
M lib/python/asterisk/sipp.py
M lib/python/asterisk/utils_socket.py
M tests/channels/pjsip/qualify/basic/test-config.yaml
M tests/channels/pjsip/qualify/max_initial_qualify_time/test-config.yaml
M tests/channels/pjsip/qualify/qualify_timeout/test-config.yaml
M tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/pjsip.conf
D tests/channels/pjsip/subscriptions/presence/presencestate_repeat/repeat_presence_state.py
M tests/channels/pjsip/subscriptions/presence/presencestate_repeat/test-config.yaml
M tests/rest_api/applications/subscribe-endpoint/nominal/tech/test-config.yaml
12 files changed, 214 insertions(+), 123 deletions(-)
Approvals:
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved
Friendly Automation: Approved for Submit
diff --git a/lib/python/asterisk/matcher_listener.py b/lib/python/asterisk/matcher_listener.py
index cd2ca5a..69c2bf1 100644
--- a/lib/python/asterisk/matcher_listener.py
+++ b/lib/python/asterisk/matcher_listener.py
@@ -74,3 +74,20 @@
if not any(f for f in self.filter_msgs if re.match(f, msg)):
self.check(msg)
+
+
+class Ari(PluggableConditions):
+ """Pluggable module that that checks messages received over ARI"""
+
+ def __init__(self, config, test_object, on_match=None):
+ """Constructor
+
+ Keyword Arguments:
+ config - configuration for this module
+ test_object - the TestCase driver
+ on_match - Optional callback called upon a conditional match
+ """
+
+ super(Ari, self).__init__(config, test_object, on_match)
+
+ test_object.register_ws_event_handler(self.check)
diff --git a/lib/python/asterisk/pluggable_modules.py b/lib/python/asterisk/pluggable_modules.py
index 22da37e..c4b0d27 100644
--- a/lib/python/asterisk/pluggable_modules.py
+++ b/lib/python/asterisk/pluggable_modules.py
@@ -17,6 +17,7 @@
from twisted.internet import reactor
from starpy import fastagi
from .test_runner import load_and_parse_module
+from .sipp import SIPpActionModule, SIPpStartEventModule
from .pluggable_registry import PLUGGABLE_ACTION_REGISTRY,\
PLUGGABLE_EVENT_REGISTRY,\
PluggableRegistry
diff --git a/lib/python/asterisk/self_test/test_utils_socket.py b/lib/python/asterisk/self_test/test_utils_socket.py
index 34672c6..b0bee5c 100755
--- a/lib/python/asterisk/self_test/test_utils_socket.py
+++ b/lib/python/asterisk/self_test/test_utils_socket.py
@@ -144,20 +144,20 @@
p = 50000
def test(h, s, f):
- self.assertEqual(get_available_port(h, s, f, 0, p), p)
+ self.assertEqual(get_available_port(h, p, s, f, 0), p)
# Limit to single host since using global object
self._on_socktype(None, test)
p = 50001
self.assertEqual(get_available_port(
- config={'-i': '0.0.0.0'}, num=2, port=p), p)
+ '0.0.0.0', p, num=2,), p)
self.assertEqual(get_available_port(
- config={'-i': '0.0.0.0', '-t': ''}, num=2, port=p), p)
+ '0.0.0.0', p, SOCK_STREAM, num=2), p)
self.assertEqual(get_available_port(
- config={'-i': '[::]'}, num=2, port=p), p)
+ '[::]', p, family=AF_INET6, num=2), p)
self.assertEqual(get_available_port(
- config={'-i': '[::]', '-t': ''}, num=2, port=p), p)
+ '[::]', p, SOCK_STREAM, AF_INET6, 2), p)
if __name__ == "__main__":
diff --git a/lib/python/asterisk/sipp.py b/lib/python/asterisk/sipp.py
index 1341132..498bf19 100644
--- a/lib/python/asterisk/sipp.py
+++ b/lib/python/asterisk/sipp.py
@@ -17,7 +17,7 @@
from .test_case import TestCase
from .utils_socket import get_available_port
from .pluggable_registry import PLUGGABLE_EVENT_REGISTRY,\
- PLUGGABLE_ACTION_REGISTRY, var_replace
+ PLUGGABLE_ACTION_REGISTRY
LOGGER = logging.getLogger(__name__)
@@ -686,7 +686,7 @@
#
# num = 4 = ports for audio rtp/rtcp and video rtp/rtcp
default_args['-mp'] = str(get_available_port(
- config=default_args, num=4))
+ default_args.get('-i'), num=4))
for (key, val) in default_args.items():
sipp_args.extend([key, val])
@@ -962,3 +962,41 @@
PLUGGABLE_EVENT_REGISTRY.register("sipp-start", SIPpStartEventModule)
+
+
+class SIPpActionModule(object):
+ """An action module that initiates SIPp scenarios."""
+
+ def __init__(self, test_object, config):
+ """Initialize SIPp action module"""
+
+ scenarios = config.get('scenarios')
+ if not scenarios:
+ # This is more of a fix the test error. Either this action
+ # is not needed, or a scenario needs to be properly added
+ LOGGER.error("No registered SIPp scenarios (action does nothing).")
+ test_object.set_passed(False)
+ test_object.stop_reactor()
+ return
+
+ self.sequence = SIPpScenarioSequence(test_object,
+ fail_on_any=config.get('fail-on-any', True),
+ stop_on_done=config.get('stop-after-scenarios', True))
+
+ for s in scenarios:
+ if ('coordinated-sender' in s and 'coordinated-receiver' in s):
+ self.sequence.register_scenario(CoordinatedScenario(
+ test_object.test_name, s))
+ else:
+ self.sequence.register_scenario(SIPpScenario(
+ test_object.test_name, s['key-args'],
+ s.get('ordered-args') or [],
+ s.get('target') or '127.0.0.1'))
+
+ def run(self, triggered_by, source, extra):
+ """Execute specified SIPp scenarios"""
+
+ self.sequence.execute()
+
+
+PLUGGABLE_ACTION_REGISTRY.register("sipp", SIPpActionModule)
diff --git a/lib/python/asterisk/utils_socket.py b/lib/python/asterisk/utils_socket.py
index 158592d..c8813bd 100644
--- a/lib/python/asterisk/utils_socket.py
+++ b/lib/python/asterisk/utils_socket.py
@@ -39,6 +39,40 @@
}.get(family, 'Unknown Family')
+def get_resolved(host='', family=None):
+ """Get and/or validate the family and host and/or resolve it"""
+
+ def get_family(h, f):
+ try:
+ inet_pton(f, h)
+ return f
+ except:
+ return None
+
+ if not host:
+ return ('', family or AF_INET)
+
+ host = host.lstrip('[').rstrip(']')
+
+ if family == None:
+ family = get_family(host, AF_INET) or get_family(host, AF_INET6)
+
+ if family:
+ # host is already an ip address, so go ahead and return
+ return (host, family)
+
+ if family == None:
+ family = AF_INET
+
+ try:
+ host = getaddrinfo(host, None, family)[0][4][0]
+ except:
+ raise ValueError("Unable to resolve host '{0}' ({1}) ".format(
+ host, socket_family(family)))
+
+ return (host, family)
+
+
def get_unused_os_port(host='', port=0, socktype=SOCK_STREAM, family=AF_INET):
"""Retrieve an unused port from the OS.
@@ -73,6 +107,9 @@
res = 0
s = socket(family, socktype)
try:
+ if socktype == SOCK_STREAM:
+ s.setsockopt(SOL_SOCKET, SO_REUSEADDR, 1)
+
s.bind((host, port))
res = s.getsockname()[1]
except error as e:
@@ -285,8 +322,8 @@
PORTS = Ports()
-def get_available_port(host='', socktype=0, family=0, num=0,
- port=0, config=None):
+def get_available_port(host='', port=0, socktype=SOCK_DGRAM,
+ family=None, num=0):
"""Retrieve the primary available port, and reserve it and its offsets.
The majority of use cases probably involve the need to reserve multiple
@@ -304,20 +341,8 @@
The singular primary available port.
"""
- if config:
- host = host or config.get('-i')
- socktype = SOCK_STREAM if '-t' in config else SOCK_DGRAM
+ host, family = get_resolved(host, family)
- # The family (without a host) and socktype should really always be
- # specified when calling this function, but just in case use some defaults
- if family < 1:
- if host:
- family = AF_INET6 if ':' in host else AF_INET
- else:
- family = AF_INET
-
- if socktype < 1:
- socktype = SOCK_DGRAM # Most tests are UDP based
available = PORTS.get_range_and_reserve(
host, port, socktype, family, num)
# If we don't have a valid socktype and family badness happens
diff --git a/tests/channels/pjsip/qualify/basic/test-config.yaml b/tests/channels/pjsip/qualify/basic/test-config.yaml
index 8e09b04..5be9ca5 100644
--- a/tests/channels/pjsip/qualify/basic/test-config.yaml
+++ b/tests/channels/pjsip/qualify/basic/test-config.yaml
@@ -11,11 +11,12 @@
modules:
-
config-section: 'ami-config-13.5'
- typename: 'ami.AMIEventModule'
+ typename: 'pluggable_modules.EventActionModule'
test-object-config:
fail-on-any: False
reactor-timeout: 10
+ stop-after-scenarios: False
test-iterations:
-
scenarios:
@@ -23,16 +24,18 @@
ami-config-13.5:
-
- type: 'headermatch'
- id: '0'
- conditions:
- match:
- Event: 'ContactStatus'
- ContactStatus: 'Reachable'
- requirements:
- match:
- URI: 'sip:127.0.0.1:5061'
- count: '>1'
+ ami-start:
+ -
+ ami-events:
+ conditions:
+ match:
+ Event: 'ContactStatus'
+ ContactStatus: 'Reachable'
+ requirements:
+ match:
+ URI: 'sip:127.0.0.1:5061'
+ count: '>1'
+ stop_test:
properties:
dependencies:
diff --git a/tests/channels/pjsip/qualify/max_initial_qualify_time/test-config.yaml b/tests/channels/pjsip/qualify/max_initial_qualify_time/test-config.yaml
index 06e43af..99bae87 100644
--- a/tests/channels/pjsip/qualify/max_initial_qualify_time/test-config.yaml
+++ b/tests/channels/pjsip/qualify/max_initial_qualify_time/test-config.yaml
@@ -18,6 +18,7 @@
connect-ami: True
fail-on-any: False
reactor-timeout: 15
+ stop-after-scenarios: False
test-iterations:
-
scenarios:
diff --git a/tests/channels/pjsip/qualify/qualify_timeout/test-config.yaml b/tests/channels/pjsip/qualify/qualify_timeout/test-config.yaml
index ad4fb55..6f97f4f 100644
--- a/tests/channels/pjsip/qualify/qualify_timeout/test-config.yaml
+++ b/tests/channels/pjsip/qualify/qualify_timeout/test-config.yaml
@@ -11,11 +11,12 @@
modules:
-
config-section: 'ami-config-13.5'
- typename: 'ami.AMIEventModule'
+ typename: 'pluggable_modules.EventActionModule'
test-object-config:
fail-on-any: False
reactor-timeout: 25
+ stop-after-scenarios: False
test-iterations:
-
scenarios:
@@ -23,16 +24,18 @@
ami-config-13.5:
-
- type: 'headermatch'
- id: '0'
- conditions:
- match:
- Event: 'ContactStatus'
- ContactStatus: 'Unreachable'
- requirements:
- match:
- URI: 'sip:127.0.0.1:5061'
- count: '>1'
+ ami-start:
+ -
+ ami-events:
+ conditions:
+ match:
+ Event: 'ContactStatus'
+ ContactStatus: 'Unreachable'
+ requirements:
+ match:
+ URI: 'sip:127.0.0.1:5061'
+ count: '>1'
+ stop_test:
properties:
dependencies:
diff --git a/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/pjsip.conf b/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/pjsip.conf
index 3e4adca..6c21810 100644
--- a/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/pjsip.conf
+++ b/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/configs/ast1/pjsip.conf
@@ -1,3 +1,7 @@
+[global]
+type=global
+debug=yes
+
[main-transport]
type=transport
bind = 127.0.0.1
diff --git a/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/repeat_presence_state.py b/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/repeat_presence_state.py
deleted file mode 100644
index 161ad5c..0000000
--- a/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/repeat_presence_state.py
+++ /dev/null
@@ -1,29 +0,0 @@
-#!/usr/bin/env python
-'''
-Copyright (C) 2014, 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.
-'''
-
-
-class RepeatPresenceState(object):
- def __init__(self, module_config, test_object):
- self.ami = None
- self.ami_message = {
- 'Action': 'SetVar',
- 'Variable': 'PRESENCE_STATE(CustomPresence:Eggs)',
- 'Value': 'AWAY,scrambled,feeling a bit sick'
- }
- test_object.register_ami_observer(self.ami_connect)
- test_object.register_scenario_started_observer(self.scenario_started)
-
- def ami_connect(self, ami):
- self.ami = ami
- # Give ourselves an initial presence
- self.ami.sendMessage(self.ami_message)
-
- def scenario_started(self, scenario):
- # Now repeat the same presence state.
- self.ami.sendMessage(self.ami_message)
diff --git a/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/test-config.yaml b/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/test-config.yaml
index 0bc6781..bb363d7 100644
--- a/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/test-config.yaml
+++ b/tests/channels/pjsip/subscriptions/presence/presencestate_repeat/test-config.yaml
@@ -17,18 +17,56 @@
- refleaks
test-modules:
- add-test-to-search-path: 'True'
test-object:
- config-section: sipp-config
- typename: 'sipp.SIPpTestCase'
+ config-section: object-config
+ typename: test_case.TestCaseModule
modules:
-
- typename: 'repeat_presence_state.RepeatPresenceState'
+ config-section: event-action-config
+ typename: pluggable_modules.EventActionModule
-sipp-config:
- reactor-timeout: 30
- fail-on-any: True
- test-iterations:
- -
+object-config:
+ reactor-timeout: 15
+ connect-ami: True
+
+event-action-config:
+ -
+ # On startup set the status
+ ami-start:
+ ami-actions:
+ action:
+ Action: SetVar
+ Variable: PRESENCE_STATE(CustomPresence:Eggs)
+ Value: AWAY,scrambled,feeling a bit sick
+ -
+ # Once the status is set issue a subscribe from the endpoint
+ ami-events:
+ conditions:
+ match:
+ Event: PresenceStatus
+ requirements:
+ match:
+ Status: away
+ Subtype: scrambled
+ Message: feeling a bit sick
+ count: '1'
+ sipp:
scenarios:
- { 'key-args': {'scenario': 'subscribe.xml', '-p': '5061'} }
+ -
+ # The test will end on completion of the SIPp scenario, but this event
+ # and subsequent action should trigger before that occurs
+ ami-events:
+ conditions:
+ match:
+ Event: TestEvent
+ State: SUBSCRIPTION_STATE_SET
+ requirements:
+ match:
+ StateText: ACTIVE
+ count: '1'
+ ami-actions:
+ action:
+ Action: SetVar
+ Variable: PRESENCE_STATE(CustomPresence:Eggs)
+ Value: AWAY,scrambled,feeling a bit sick
diff --git a/tests/rest_api/applications/subscribe-endpoint/nominal/tech/test-config.yaml b/tests/rest_api/applications/subscribe-endpoint/nominal/tech/test-config.yaml
index e8b2976..b677f7d 100644
--- a/tests/rest_api/applications/subscribe-endpoint/nominal/tech/test-config.yaml
+++ b/tests/rest_api/applications/subscribe-endpoint/nominal/tech/test-config.yaml
@@ -28,10 +28,7 @@
config-section: subscriber
typename: 'subscriber.ResourceSubscription'
-
- config-section: ari-config
- typename: ari.WebSocketEventModule
- -
- config-section: ari-test-stopper
+ config-section: event-action-config
typename: pluggable_modules.EventActionModule
test-object-config:
@@ -58,19 +55,11 @@
subscriptions:
- { event-source: 'endpoint:IAX2', app: 'testsuite' }
-ari-config:
- events:
- - conditions:
- match:
- type: EndpointStateChange
- application: testsuite
- endpoint:
- technology: IAX2
- resource: alice
- state: unknown
- channel_ids: ['.*']
- count: 1
- - conditions:
+event-action-config:
+ event:
+ type: 'matcher_listener.Ari'
+ conditions:
+ -
match:
type: EndpointStateChange
application: testsuite
@@ -79,8 +68,17 @@
resource: bob
state: online
channel_ids: ['.*']
- count: 0
- - conditions:
+ count: 0
+ -
+ match:
+ type: EndpointStateChange
+ application: testsuite
+ endpoint:
+ technology: IAX2
+ resource: alice
+ state: unknown
+ channel_ids: ['.*']
+ -
match:
type: ChannelCreated
application: testsuite
@@ -88,32 +86,28 @@
name: 'IAX2/alice-.*'
state: Down
dialplan: { context: 'default', exten: 's', priority: 1 }
- count: 1
- - conditions:
+ -
match:
type: ChannelStateChange
application: testsuite
channel:
name: 'IAX2/alice-.*'
state: Ringing
- count: 1
- - conditions:
+ -
match:
type: ChannelStateChange
application: testsuite
channel:
name: 'IAX2/alice-.*'
state: Up
- count: 1
- - conditions:
+ -
match:
type: ChannelHangupRequest
application: testsuite
channel:
name: 'IAX2/alice-.*'
state: Up
- count: 1
- - conditions:
+ -
match:
type: ChannelDestroyed
application: testsuite
@@ -122,20 +116,16 @@
channel:
name: 'IAX2/alice-.*'
state: Up
- count: 1
-
-ari-test-stopper:
- -
- ari-events:
- match:
- type: EndpointStateChange
- application: testsuite
- endpoint:
- technology: IAX2
- resource: alice
- state: unknown
- channel_ids: []
- stop_test:
+ -
+ match:
+ type: EndpointStateChange
+ application: testsuite
+ endpoint:
+ technology: IAX2
+ resource: alice
+ state: unknown
+ channel_ids: []
+ stop_test:
properties:
dependencies:
--
To view, visit https://gerrit.asterisk.org/c/testsuite/+/18433
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: testsuite
Gerrit-Branch: certified/18.9
Gerrit-Change-Id: I5d8bb49ba8c8238ba76fd2545ff9be2ff0154399
Gerrit-Change-Number: 18433
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220426/9f74e745/attachment-0001.html>
More information about the asterisk-code-review
mailing list