<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/4090/">https://reviewboard.asterisk.org/r/4090/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 18th, 2014, 9:18 a.m. EST, <b>Corey Farrell</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Please discard my findings, they are minor. I'd like to see this committed ASAP so further improvements can be made against it.</pre>
</blockquote>
<p>On November 18th, 2014, 10:41 a.m. EST, <b>Scott Griepentrog</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I like your idea of adding an option to pass valgrind options, but it needs to first be set to write logs to the appropriate run_x/asty directory so there isn't a conflict between multiple asterisk instances. Then a simple parser added to check the log for critical issues that trigger a test failure, which then get included in the test results, even if in an abbreviated form. Not sure how best to pass leak info other than just leaving it in the log though.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Upon further thought, I'm not sure it's desirable to be able to add options through the command-line. "Extra options read from ~/.valgrindrc, $VALGRIND_OPTS, ./.valgrindrc". So I think in most cases it would be best for people to create ./.valgrindrc. Maybe we can add that to svn:ignore for '.'. But as you said, getting the logs to run_x/asty dirs is more important. Once this change is submitted I will look at putting the logs in the appropriate dir.
As for parsing the test results, I think this would require using --xml-file=, then using libxml and/or libxslt (through python, sorry I don't know the modules for this). Anything we do to parse the text-based valgrind output will be a fragile hack.</pre>
<br />
<p>- Corey</p>
<br />
<p>On November 18th, 2014, 10:49 a.m. EST, Scott Griepentrog wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Scott Griepentrog.</div>
<p style="color: grey;"><i>Updated Nov. 18, 2014, 10:49 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
testsuite
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This adds very basic valgrind support, which is convenient for manual test runs but does not (yet) include support for more automated valgrind usage.
1) CLI flag '-V' enables valgrind (./runtests -V -t tests/test)
2) Minimal changes to testsuite, other improvements can be added later if desired
3) Valgrind output is picked up by error logging and shown after test run.
4) Unlike previous valgrind attempt, this one works fine on tests with multiple asterisk instances
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/asterisk/trunk/runtests.py <span style="color: grey">(5733)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/test_case.py <span style="color: grey">(5733)</span></li>
<li>/asterisk/trunk/lib/python/asterisk/asterisk.py <span style="color: grey">(5733)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4090/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>