[Asterisk-code-review] Testsuite: Corrected Order of Operations for RLSTest Ctor (testsuite[master])
Anonymous Coward
asteriskteam at digium.com
Wed Nov 18 16:21:30 CST 2015
Anonymous Coward #1000019 has submitted this change and it was merged.
Change subject: Testsuite: Corrected Order of Operations for RLSTest Ctor
......................................................................
Testsuite: Corrected Order of Operations for RLSTest Ctor
Because of the order of operations in the initializer, it
was possible to access a class member (self.token) before it
was declared in the initializer (via
self.check_prerequisites.)
This defect was only made noticeable however, because the
condition that the prerequisites check itself was examining
was incorrect. (The condition was only checking to see if an
ami-action attribute was present in the test module's
test-config.yaml file without also considering if the
test-object declared in the test module's test-config.yaml
file was of a type that could support responding to ami
actions. So, essentially, any test module configuration
that declared an ami-action attribute would have failed.
This is, in fact, what exposted the defect with the
initialization order for the RLSTest members.)
For more information, please refer to the Jenkins incident
(a link for which is also included in the original ASTERISK
issue for your convenience):
"https://jenkins.asterisk.org/jenkins/job/periodic-asterisk-1
3/lastSuccessfulBuild/testReport/(root)/AsteriskTestSuite/te
sts_channels_pjsip_subscriptions_rls_lists_nominal_presence_
full_state/"
ASTERISK-25556
Reported By: Ashley Sanders
Change-Id: I9c2e41d3ae56594f426cc0b23fc5ca808b856ddb
---
M tests/channels/pjsip/subscriptions/rls/rls_test.py
1 file changed, 27 insertions(+), 21 deletions(-)
Approvals:
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, approved
diff --git a/tests/channels/pjsip/subscriptions/rls/rls_test.py b/tests/channels/pjsip/subscriptions/rls/rls_test.py
index 9d41fbe..b4cadc0 100755
--- a/tests/channels/pjsip/subscriptions/rls/rls_test.py
+++ b/tests/channels/pjsip/subscriptions/rls/rls_test.py
@@ -2,6 +2,7 @@
"""
Copyright (C) 2015, Digium, Inc.
Jonathan Rose <jrose at digium.com>
+Ashley Sanders <asanders at digium.com>
This program is free software, distributed under the terms of
the GNU General Public License Version 2.
@@ -39,7 +40,7 @@
# If the packet body is not a multipart packet body, this is not a
# packet we care about.
if packet.body.packet_type != "Multipart":
- LOGGER.debug("Ignoring packet, NOTIFY does not contain " \
+ LOGGER.debug("Ignoring packet, NOTIFY does not contain "
"multipart body")
return False
@@ -108,9 +109,10 @@
self.test_object = test_object
self.ami_action = module_config.get("ami-action")
- if self.check_prerequisites():
- self.test_object.register_scenario_started_observer(
- self.on_scenario_started)
+ token_msg = (
+ "Test execution was interrupted before all NOTIFY "
+ "packets were accounted for.")
+ self.token = self.test_object.create_fail_token(token_msg)
self.ami = None
self.packets_idx = 0
@@ -121,12 +123,12 @@
self.stop_test_after_notifys = \
module_config.get("stop-test-after-notifys", True)
- token_msg = "Test execution was interrupted before all NOTIFY " \
- "packets were accounted for."
- self.token = self.test_object.create_fail_token(token_msg)
-
self.test_object.register_ami_observer(self.on_ami_connect)
self.add_callback("SIP", self.multipart_handler)
+
+ if self.check_prerequisites():
+ self.test_object.register_scenario_started_observer(
+ self.on_scenario_started)
def check_prerequisites(self):
"""Checks the test_object can support an ami_action, if configured.
@@ -142,16 +144,18 @@
is_start_observer = hasattr(self.test_object,
"register_scenario_started_observer")
- if self.ami_action is not None:
- message = "This test is configured with an ami_action " \
- "attribute. However, it is also configured to " \
- "use a test-object that does not contain support " \
- "for 'register_scenario_started_observer'. As a " \
- "result, the ami_action will never be executed. " \
- "Either reconfigure the test to run without a " \
- "dependency for an ami_action or change the " \
- "test-object type to a type that supports " \
- "'register_scenario_started_observer'."
+
+ if not is_start_observer and self.ami_action is not None:
+ message = (
+ "This test is configured with an ami_action "
+ "attribute. However, it is also configured to "
+ "use a test-object that does not contain support "
+ "for 'register_scenario_started_observer'. As a "
+ "result, the ami_action will never be executed. "
+ "Either reconfigure the test to run without a "
+ "dependency for an ami_action or change the "
+ "test-object type to a type that supports "
+ "'register_scenario_started_observer'.")
self.fail_test(message)
return is_start_observer
@@ -186,9 +190,10 @@
return
if self.packets_idx >= len(self.packets):
- message = "Received more packets ({0}) than expected ({1}). " \
- "Failing test.".format(self.packets_idx,
- len(self.packets))
+ message = (
+ "Received more packets ({0}) than expected ({1}). "
+ "Failing test.").format(self.packets_idx,
+ len(self.packets))
self.fail_test(message)
rls_packet = RLSPacket(packet)
@@ -225,6 +230,7 @@
def on_ami_connect(self, ami):
"""Callback when AMI connects. Sets test AMI instance."""
+
self.ami = ami
def on_scenario_started(self, scenario):
--
To view, visit https://gerrit.asterisk.org/1635
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9c2e41d3ae56594f426cc0b23fc5ca808b856ddb
Gerrit-PatchSet: 3
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
More information about the asterisk-code-review
mailing list