[Asterisk-code-review] Add support for delayed shutdown of Asterisk when checking f... (testsuite[master])

Jenkins2 asteriskteam at digium.com
Wed Nov 1 09:54:57 CDT 2017


Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/6845 )

Change subject: Add support for delayed shutdown of Asterisk when checking for leaks.
......................................................................

Add support for delayed shutdown of Asterisk when checking for leaks.

Add memcheck-delay-stop option to test-config.yaml in the test-object
config section.  This causes a pause during shutdown if Asterisk is run
under valgrind or has REF_DEBUG enabled.  This delay is not meant to
compensate for slowness caused by memory debugging, but to allow SIP
timers to clean resources before shutdown of Asterisk is initiated.

If set this options forces reactor-timeout to be at least 3 seconds
longer.  This is only done to stop the testsuite from printing a reactor
timeout warning for tests with a large memcheck-delay-stop value.

I had considered setting a non-zero default for memcheck-delay-stop but
the majority of tests do not require any delay.  Since the testsuite
contains nearly 1000 tests using a 7 second default delay would add
nearly 2 hours to the total runtime.  I estimate that only 20% of the
tests require any delay.  The amount of delay needed varies based on
which timers we need to wait for: timer_b, session expiration, defered
session termination.  The minimums for these timers range from 6.4 - 90
seconds.  In most cases using the minimum delay will require modification
of the test.  For example reconfiguring timer_t1 and timer_b in some basic
PJSIP call tests or reducing the 'Expires:' header in SIP SUBSCRIBE tests.

As an example of usage tests/channels/pjsip/accountcode has been
updated.  The timer_t1 and timer_b have been decreased to minimize the
delay needed.

ASTERISK-27306

Change-Id: I67ec89e4a116526ad34796a233e15fec794cf3e1
---
M lib/python/asterisk/apptest.py
M lib/python/asterisk/ari.py
M lib/python/asterisk/asterisk.py
M lib/python/asterisk/sipp.py
M lib/python/asterisk/test_case.py
M tests/channels/pjsip/accountcode/configs/ast1/pjsip.conf
M tests/channels/pjsip/accountcode/test-config.yaml
7 files changed, 48 insertions(+), 22 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/lib/python/asterisk/apptest.py b/lib/python/asterisk/apptest.py
index dc982b5..77d74e2 100644
--- a/lib/python/asterisk/apptest.py
+++ b/lib/python/asterisk/apptest.py
@@ -71,7 +71,7 @@
 
         self.register_ami_observer(self._ami_connect_handler)
         self.register_stop_observer(self.end_scenario)
-        self.create_asterisk()
+        self.create_asterisk(test_config=test_config)
 
         # Created successfully - set the singleton instance to this object
         # if we're the first instance created; otherwise, complain loudly
diff --git a/lib/python/asterisk/ari.py b/lib/python/asterisk/ari.py
index 6241f13..da19cee 100644
--- a/lib/python/asterisk/ari.py
+++ b/lib/python/asterisk/ari.py
@@ -84,7 +84,7 @@
         self.timed_out = False
 
         self.asterisk_instances = test_config.get('asterisk-instances', 1)
-        self.create_asterisk(count=self.asterisk_instances)
+        self.create_asterisk(count=self.asterisk_instances, test_config=test_config)
 
     def run(self):
         """Override of TestCase run
diff --git a/lib/python/asterisk/asterisk.py b/lib/python/asterisk/asterisk.py
index 2302d54..1c2da02 100755
--- a/lib/python/asterisk/asterisk.py
+++ b/lib/python/asterisk/asterisk.py
@@ -341,7 +341,7 @@
         default_etc_directory = "/etc/asterisk"
 
     def __init__(self, base=None, ast_conf_options=None, host="127.0.0.1",
-                 remote_config=None):
+                 remote_config=None, test_config=None):
         """Construct an Asterisk instance.
 
         Keyword Arguments:
@@ -356,6 +356,7 @@
                          ast_conf_options are generally ignored, and the
                          Asterisk instance's configuration is treated as
                          immutable on some remote machine defined by 'host'
+        test_config -- yaml loaded object containing config information
 
         Example Usage:
         self.asterisk = Asterisk(base="manager/login")
@@ -370,6 +371,9 @@
         self.original_astmoddir = ""
         self.ast_version = None
         self.remote_config = remote_config
+        self.memcheck_delay_stop = 0
+        if test_config is not None and 'memcheck-delay-stop' in test_config:
+            self.memcheck_delay_stop = test_config['memcheck-delay-stop'] or 0
 
         # If the process is remote, don't bother
         if not self.remote_config:
@@ -622,11 +626,22 @@
             self._stop_deferred.callback(reason)
             return reason
 
+        def __actual_stop():
+            # Schedule a kill. If we don't gracefully shut down Asterisk, this
+            # will ensure that the test is stopped.
+            sched_time = 200 if self.valgrind_enabled else 45
+
+            self._stop_cancel_tokens.append(reactor.callLater(sched_time,
+                                            __send_kill))
+
+            # Start by asking to stop gracefully.
+            __send_stop_gracefully()
+
+            self._stop_deferred.addCallback(__cancel_stops)
+
         if not self.process:
             reactor.callLater(0, __process_stopped, None)
-            return self._stop_deferred
-
-        if self.protocol.exited:
+        elif self.protocol.exited:
             try:
                 if not self._stop_deferred.called:
                     self._stop_deferred.callback(
@@ -635,16 +650,13 @@
                 LOGGER.warning("Asterisk %s stop deferred already called" %
                                self.host)
         else:
-            # Schedule a kill. If we don't gracefully shut down Asterisk, this
-            # will ensure that the test is stopped.
-            sched_time = 200 if self.valgrind_enabled else 45
-            self._stop_cancel_tokens.append(reactor.callLater(sched_time,
-                                            __send_kill))
+            delay = 0
+            if self.valgrind_enabled or os.path.exists(self.get_path("astlogdir", 'refs')):
+                delay = self.memcheck_delay_stop
+                if delay != 0:
+                    LOGGER.debug("Delaying shutdown by %d seconds" % delay)
 
-            # Start by asking to stop gracefully.
-            __send_stop_gracefully()
-
-            self._stop_deferred.addCallback(__cancel_stops)
+            reactor.callLater(delay, __actual_stop)
 
         return self._stop_deferred
 
diff --git a/lib/python/asterisk/sipp.py b/lib/python/asterisk/sipp.py
index 6b4c811..acdb20d 100644
--- a/lib/python/asterisk/sipp.py
+++ b/lib/python/asterisk/sipp.py
@@ -103,7 +103,7 @@
             self._stop_after_scenarios = True
 
         self.register_intermediate_obverver(self._handle_scenario_finished)
-        self.create_asterisk()
+        self.create_asterisk(test_config=test_config)
 
     def on_reactor_timeout(self):
         """Create a failure token when the test times out"""
diff --git a/lib/python/asterisk/test_case.py b/lib/python/asterisk/test_case.py
index 815c4c8..ec5e09d 100644
--- a/lib/python/asterisk/test_case.py
+++ b/lib/python/asterisk/test_case.py
@@ -154,6 +154,11 @@
                 self.base_config_path = test_config['config-path']
             if 'reactor-timeout' in test_config:
                 self.reactor_timeout = test_config['reactor-timeout']
+            if 'memcheck-delay-stop' in test_config:
+                # minimum reactor timeout 3 seconds more than memcheck-delay-stop.
+                delay = test_config['memcheck-delay-stop'] + 3
+                if delay > self.reactor_timeout:
+                    self.reactor_timeout = delay
             self.ast_conf_options = test_config.get('ast-config-options')
             log_full = test_config.get('log-full', True)
             log_messages = test_config.get('log-messages', True)
@@ -246,7 +251,7 @@
                          for i in range(count)]
         return asterisks
 
-    def create_asterisk(self, count=1, base_configs_path=None):
+    def create_asterisk(self, count=1, base_configs_path=None, test_config=None):
         """Create n instances of Asterisk
 
         Note: if the instances of Asterisk being created are remote, the
@@ -261,6 +266,7 @@
                           the same configuration all the time. This
                           configuration can be overwritten by individual tests,
                           however.
+        test_config       Test Configuration
         """
         for i, ast_config in enumerate(self.get_asterisk_hosts(count)):
             local_num = ast_config.get('num')
@@ -273,11 +279,13 @@
             if local_num:
                 LOGGER.info("Creating Asterisk instance %d" % local_num)
                 ast_instance = Asterisk(base=self.testlogdir, host=host,
-                                        ast_conf_options=self.ast_conf_options)
+                                        ast_conf_options=self.ast_conf_options,
+                                        test_config=test_config)
             else:
                 LOGGER.info("Managing Asterisk instance at %s" % host)
                 ast_instance = Asterisk(base=self.testlogdir, host=host,
-                                        remote_config=ast_config)
+                                        remote_config=ast_config,
+                                        test_config=test_config)
             self.ast.append(ast_instance)
             self.condition_controller.register_asterisk_instance(self.ast[i])
 
@@ -293,7 +301,7 @@
                 # Copy test specific config files
                 self.ast[i].install_configs("%s/configs/ast%d" %
                                             (self.test_name, local_num),
-                                            self.test_config.get_deps())
+                                            self.test_config)
 
     def create_ami_factory(self, count=1, username="user", secret="mysecret",
                            port=5038):
@@ -816,7 +824,7 @@
             self._end_test_delay = test_config.get('end-test-delay') or 0
             self._stop_on_end = test_config.get('stop-on-end', True)
 
-        self.create_asterisk(count=1)
+        self.create_asterisk(count=1, test_config=test_config)
 
     def ami_connect(self, ami):
         """AMI connect handler"""
@@ -1000,7 +1008,7 @@
         self.connect_ami = test_config.get('connect-ami') or False
         self.connect_agi = test_config.get('connect-agi') or False
 
-        self.create_asterisk(count=self.asterisk_instances)
+        self.create_asterisk(count=self.asterisk_instances, test_config=test_config)
 
     def run(self):
         """The reactor entry point"""
diff --git a/tests/channels/pjsip/accountcode/configs/ast1/pjsip.conf b/tests/channels/pjsip/accountcode/configs/ast1/pjsip.conf
index 5450c11..384b78f 100644
--- a/tests/channels/pjsip/accountcode/configs/ast1/pjsip.conf
+++ b/tests/channels/pjsip/accountcode/configs/ast1/pjsip.conf
@@ -1,3 +1,8 @@
+[system]
+type=system
+timer_t1=100
+timer_b=6400
+
 [local-transport]
 type=transport
 bind=127.0.0.1
diff --git a/tests/channels/pjsip/accountcode/test-config.yaml b/tests/channels/pjsip/accountcode/test-config.yaml
index c6679e5..ec075f5 100644
--- a/tests/channels/pjsip/accountcode/test-config.yaml
+++ b/tests/channels/pjsip/accountcode/test-config.yaml
@@ -16,6 +16,7 @@
             typename: 'ami.AMIEventModule'
 
 test-object-config:
+    memcheck-delay-stop: 7
     spawn-after-hangup: True
     test-iterations:
         -

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

Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I67ec89e4a116526ad34796a233e15fec794cf3e1
Gerrit-Change-Number: 6845
Gerrit-PatchSet: 4
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171101/9a036ca7/attachment-0001.html>


More information about the asterisk-code-review mailing list