[asterisk-dev] [Code Review] 3496: testsuite: add valgrind support

[Corey Farrell]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3496/#review12061
-----------------------------------------------------------



/asterisk/trunk/lib/python/asterisk/test_case.py
<https://reviewboard.asterisk.org/r/3496/#comment22034>

    I don't think this message is needed/useful.



/asterisk/trunk/lib/python/asterisk/valgrind.py
<https://reviewboard.asterisk.org/r/3496/#comment22039>

    This should be default 1, and only set to 20 after we confirm valgrind is enabled.  This way instead of using conditionals (if self.valgrind.enable: delay *= self.valgrind.multiplier) you can just blindly multiply by self.valgrind.multiplier.
    
    On a side-note, I think it would be good if we could use some kind of global option (not specific to valgrind support) for debug_multiplier.  This way the single variable could be multiplied for any number of debug tools or slow build options, and all code would just use the single multiplier.  I'm not sure where a global debug_multiplier would be set.



/asterisk/trunk/lib/python/asterisk/valgrind.py
<https://reviewboard.asterisk.org/r/3496/#comment22036>

    As someone mentioned in your first review for valgrind support, XSLT would be better.  I had started writing an XSLT file to convert the results to test output before, I'll have to take another look at it.  If you want to open a JIRA ticket for this I can upload what I have so far.



/asterisk/trunk/runtests.py
<https://reviewboard.asterisk.org/r/3496/#comment22031>

    I think a message could be good even when the test was already failed.  Leaks that happen due to error's are just as important as leaks on success.



/asterisk/trunk/runtests.py
<https://reviewboard.asterisk.org/r/3496/#comment22032>

    I think maybe VALGRINDENABLE would be less likely to have conflicts.  Maybe I spend too much time in build systems but I would expect that if we use environment VALGRIND it should be to allow an alternative valgrind binary:
    
    VALGRIND=/usr/local/bin/valgrind ./runtests.py -V
    
    This would be if for whatever reason /usr/bin/valgrind provided by the distribution is undesirable.



/asterisk/trunk/tests/rest_api/tests.yaml
<https://reviewboard.asterisk.org/r/3496/#comment22035>

    Unrelated change?


- Corey Farrell


On April 29, 2014, 2:27 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3496/
> -----------------------------------------------------------
> 
> (Updated April 29, 2014, 2:27 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> This patch adds support for running Asterisk under Valgrind (said "Val-Grinned") to check for memory allocation and reference problems.
> 
> To use, add -V to ./runtests.py and get a cup of coffee.  Any errors will be displayed in a stack trace format.
> 
> Tests can also activate valgrind in the yaml configuration.
> 
> By default, only confirmed leaks will be shown - thoses where all copies of the pointer to the allocation have been released.  Additional leak checking can be enabled.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/rest_api/tests.yaml 5006 
>   /asterisk/trunk/runtests.py 5006 
>   /asterisk/trunk/logger.conf 5006 
>   /asterisk/trunk/lib/python/asterisk/valgrind.py PRE-CREATION 
>   /asterisk/trunk/lib/python/asterisk/test_config.py 5006 
>   /asterisk/trunk/lib/python/asterisk/test_case.py 5006 
>   /asterisk/trunk/lib/python/asterisk/asterisk.py 5006 
>   /asterisk/trunk/configs/valgrind.supp PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3496/diff/
> 
> 
> Testing
> -------
> 
> Used to locate reference problems on Userevent issue, and incorporated several improvements over the prior attempt.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140605/5dcaff8d/attachment.html>