<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6845">View Change</a></p><div style="white-space:pre-wrap">Approvals:
George Joseph: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Add support for delayed shutdown of Asterisk when checking for leaks.<br><br>Add memcheck-delay-stop option to test-config.yaml in the test-object<br>config section. This causes a pause during shutdown if Asterisk is run<br>under valgrind or has REF_DEBUG enabled. This delay is not meant to<br>compensate for slowness caused by memory debugging, but to allow SIP<br>timers to clean resources before shutdown of Asterisk is initiated.<br><br>If set this options forces reactor-timeout to be at least 3 seconds<br>longer. This is only done to stop the testsuite from printing a reactor<br>timeout warning for tests with a large memcheck-delay-stop value.<br><br>I had considered setting a non-zero default for memcheck-delay-stop but<br>the majority of tests do not require any delay. Since the testsuite<br>contains nearly 1000 tests using a 7 second default delay would add<br>nearly 2 hours to the total runtime. I estimate that only 20% of the<br>tests require any delay. The amount of delay needed varies based on<br>which timers we need to wait for: timer_b, session expiration, defered<br>session termination. The minimums for these timers range from 6.4 - 90<br>seconds. In most cases using the minimum delay will require modification<br>of the test. For example reconfiguring timer_t1 and timer_b in some basic<br>PJSIP call tests or reducing the 'Expires:' header in SIP SUBSCRIBE tests.<br><br>As an example of usage tests/channels/pjsip/accountcode has been<br>updated. The timer_t1 and timer_b have been decreased to minimize the<br>delay needed.<br><br>ASTERISK-27306<br><br>Change-Id: I67ec89e4a116526ad34796a233e15fec794cf3e1<br>---<br>M lib/python/asterisk/apptest.py<br>M lib/python/asterisk/ari.py<br>M lib/python/asterisk/asterisk.py<br>M lib/python/asterisk/sipp.py<br>M lib/python/asterisk/test_case.py<br>M tests/channels/pjsip/accountcode/configs/ast1/pjsip.conf<br>M tests/channels/pjsip/accountcode/test-config.yaml<br>7 files changed, 48 insertions(+), 22 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/lib/python/asterisk/apptest.py b/lib/python/asterisk/apptest.py<br>index dc982b5..77d74e2 100644<br>--- a/lib/python/asterisk/apptest.py<br>+++ b/lib/python/asterisk/apptest.py<br>@@ -71,7 +71,7 @@<br> <br> self.register_ami_observer(self._ami_connect_handler)<br> self.register_stop_observer(self.end_scenario)<br>- self.create_asterisk()<br>+ self.create_asterisk(test_config=test_config)<br> <br> # Created successfully - set the singleton instance to this object<br> # if we're the first instance created; otherwise, complain loudly<br>diff --git a/lib/python/asterisk/ari.py b/lib/python/asterisk/ari.py<br>index 6241f13..da19cee 100644<br>--- a/lib/python/asterisk/ari.py<br>+++ b/lib/python/asterisk/ari.py<br>@@ -84,7 +84,7 @@<br> self.timed_out = False<br> <br> self.asterisk_instances = test_config.get('asterisk-instances', 1)<br>- self.create_asterisk(count=self.asterisk_instances)<br>+ self.create_asterisk(count=self.asterisk_instances, test_config=test_config)<br> <br> def run(self):<br> """Override of TestCase run<br>diff --git a/lib/python/asterisk/asterisk.py b/lib/python/asterisk/asterisk.py<br>index 2302d54..1c2da02 100755<br>--- a/lib/python/asterisk/asterisk.py<br>+++ b/lib/python/asterisk/asterisk.py<br>@@ -341,7 +341,7 @@<br> default_etc_directory = "/etc/asterisk"<br> <br> def __init__(self, base=None, ast_conf_options=None, host="127.0.0.1",<br>- remote_config=None):<br>+ remote_config=None, test_config=None):<br> """Construct an Asterisk instance.<br> <br> Keyword Arguments:<br>@@ -356,6 +356,7 @@<br> ast_conf_options are generally ignored, and the<br> Asterisk instance's configuration is treated as<br> immutable on some remote machine defined by 'host'<br>+ test_config -- yaml loaded object containing config information<br> <br> Example Usage:<br> self.asterisk = Asterisk(base="manager/login")<br>@@ -370,6 +371,9 @@<br> self.original_astmoddir = ""<br> self.ast_version = None<br> self.remote_config = remote_config<br>+ self.memcheck_delay_stop = 0<br>+ if test_config is not None and 'memcheck-delay-stop' in test_config:<br>+ self.memcheck_delay_stop = test_config['memcheck-delay-stop'] or 0<br> <br> # If the process is remote, don't bother<br> if not self.remote_config:<br>@@ -622,11 +626,22 @@<br> self._stop_deferred.callback(reason)<br> return reason<br> <br>+ def __actual_stop():<br>+ # Schedule a kill. If we don't gracefully shut down Asterisk, this<br>+ # will ensure that the test is stopped.<br>+ sched_time = 200 if self.valgrind_enabled else 45<br>+<br>+ self._stop_cancel_tokens.append(reactor.callLater(sched_time,<br>+ __send_kill))<br>+<br>+ # Start by asking to stop gracefully.<br>+ __send_stop_gracefully()<br>+<br>+ self._stop_deferred.addCallback(__cancel_stops)<br>+<br> if not self.process:<br> reactor.callLater(0, __process_stopped, None)<br>- return self._stop_deferred<br>-<br>- if self.protocol.exited:<br>+ elif self.protocol.exited:<br> try:<br> if not self._stop_deferred.called:<br> self._stop_deferred.callback(<br>@@ -635,16 +650,13 @@<br> LOGGER.warning("Asterisk %s stop deferred already called" %<br> self.host)<br> else:<br>- # Schedule a kill. If we don't gracefully shut down Asterisk, this<br>- # will ensure that the test is stopped.<br>- sched_time = 200 if self.valgrind_enabled else 45<br>- self._stop_cancel_tokens.append(reactor.callLater(sched_time,<br>- __send_kill))<br>+ delay = 0<br>+ if self.valgrind_enabled or os.path.exists(self.get_path("astlogdir", 'refs')):<br>+ delay = self.memcheck_delay_stop<br>+ if delay != 0:<br>+ LOGGER.debug("Delaying shutdown by %d seconds" % delay)<br> <br>- # Start by asking to stop gracefully.<br>- __send_stop_gracefully()<br>-<br>- self._stop_deferred.addCallback(__cancel_stops)<br>+ reactor.callLater(delay, __actual_stop)<br> <br> return self._stop_deferred<br> <br>diff --git a/lib/python/asterisk/sipp.py b/lib/python/asterisk/sipp.py<br>index 6b4c811..acdb20d 100644<br>--- a/lib/python/asterisk/sipp.py<br>+++ b/lib/python/asterisk/sipp.py<br>@@ -103,7 +103,7 @@<br> self._stop_after_scenarios = True<br> <br> self.register_intermediate_obverver(self._handle_scenario_finished)<br>- self.create_asterisk()<br>+ self.create_asterisk(test_config=test_config)<br> <br> def on_reactor_timeout(self):<br> """Create a failure token when the test times out"""<br>diff --git a/lib/python/asterisk/test_case.py b/lib/python/asterisk/test_case.py<br>index 815c4c8..ec5e09d 100644<br>--- a/lib/python/asterisk/test_case.py<br>+++ b/lib/python/asterisk/test_case.py<br>@@ -154,6 +154,11 @@<br> self.base_config_path = test_config['config-path']<br> if 'reactor-timeout' in test_config:<br> self.reactor_timeout = test_config['reactor-timeout']<br>+ if 'memcheck-delay-stop' in test_config:<br>+ # minimum reactor timeout 3 seconds more than memcheck-delay-stop.<br>+ delay = test_config['memcheck-delay-stop'] + 3<br>+ if delay > self.reactor_timeout:<br>+ self.reactor_timeout = delay<br> self.ast_conf_options = test_config.get('ast-config-options')<br> log_full = test_config.get('log-full', True)<br> log_messages = test_config.get('log-messages', True)<br>@@ -246,7 +251,7 @@<br> for i in range(count)]<br> return asterisks<br> <br>- def create_asterisk(self, count=1, base_configs_path=None):<br>+ def create_asterisk(self, count=1, base_configs_path=None, test_config=None):<br> """Create n instances of Asterisk<br> <br> Note: if the instances of Asterisk being created are remote, the<br>@@ -261,6 +266,7 @@<br> the same configuration all the time. This<br> configuration can be overwritten by individual tests,<br> however.<br>+ test_config Test Configuration<br> """<br> for i, ast_config in enumerate(self.get_asterisk_hosts(count)):<br> local_num = ast_config.get('num')<br>@@ -273,11 +279,13 @@<br> if local_num:<br> LOGGER.info("Creating Asterisk instance %d" % local_num)<br> ast_instance = Asterisk(base=self.testlogdir, host=host,<br>- ast_conf_options=self.ast_conf_options)<br>+ ast_conf_options=self.ast_conf_options,<br>+ test_config=test_config)<br> else:<br> LOGGER.info("Managing Asterisk instance at %s" % host)<br> ast_instance = Asterisk(base=self.testlogdir, host=host,<br>- remote_config=ast_config)<br>+ remote_config=ast_config,<br>+ test_config=test_config)<br> self.ast.append(ast_instance)<br> self.condition_controller.register_asterisk_instance(self.ast[i])<br> <br>@@ -293,7 +301,7 @@<br> # Copy test specific config files<br> self.ast[i].install_configs("%s/configs/ast%d" %<br> (self.test_name, local_num),<br>- self.test_config.get_deps())<br>+ self.test_config)<br> <br> def create_ami_factory(self, count=1, username="user", secret="mysecret",<br> port=5038):<br>@@ -816,7 +824,7 @@<br> self._end_test_delay = test_config.get('end-test-delay') or 0<br> self._stop_on_end = test_config.get('stop-on-end', True)<br> <br>- self.create_asterisk(count=1)<br>+ self.create_asterisk(count=1, test_config=test_config)<br> <br> def ami_connect(self, ami):<br> """AMI connect handler"""<br>@@ -1000,7 +1008,7 @@<br> self.connect_ami = test_config.get('connect-ami') or False<br> self.connect_agi = test_config.get('connect-agi') or False<br> <br>- self.create_asterisk(count=self.asterisk_instances)<br>+ self.create_asterisk(count=self.asterisk_instances, test_config=test_config)<br> <br> def run(self):<br> """The reactor entry point"""<br>diff --git a/tests/channels/pjsip/accountcode/configs/ast1/pjsip.conf b/tests/channels/pjsip/accountcode/configs/ast1/pjsip.conf<br>index 5450c11..384b78f 100644<br>--- a/tests/channels/pjsip/accountcode/configs/ast1/pjsip.conf<br>+++ b/tests/channels/pjsip/accountcode/configs/ast1/pjsip.conf<br>@@ -1,3 +1,8 @@<br>+[system]<br>+type=system<br>+timer_t1=100<br>+timer_b=6400<br>+<br> [local-transport]<br> type=transport<br> bind=127.0.0.1<br>diff --git a/tests/channels/pjsip/accountcode/test-config.yaml b/tests/channels/pjsip/accountcode/test-config.yaml<br>index c6679e5..ec075f5 100644<br>--- a/tests/channels/pjsip/accountcode/test-config.yaml<br>+++ b/tests/channels/pjsip/accountcode/test-config.yaml<br>@@ -16,6 +16,7 @@<br> typename: 'ami.AMIEventModule'<br> <br> test-object-config:<br>+ memcheck-delay-stop: 7<br> spawn-after-hangup: True<br> test-iterations:<br> -<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6845">change 6845</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6845"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: testsuite </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I67ec89e4a116526ad34796a233e15fec794cf3e1 </div>
<div style="display:none"> Gerrit-Change-Number: 6845 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>