[asterisk-dev] [Code Review] 2646: Add CEL unit tests and do some cleanup

[Matt Jordan]

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

Ship it!


These are all minor cleanup issues.


trunk/include/asterisk/cel.h
<https://reviewboard.asterisk.org/r/2646/#comment17757>

    We need to be careful here.
    
    Exposing ao2_containers via a header file is dangerous. Consumers of this API have no idea how the container should be used, what types are stored in there, how to look them up, etc. I'd prefer it if we didn't have to expose this, but I understand why you are here (the same reason I made the CDR configuration public).
    
    At the very least, this needs to have explicit documentation on what is stored in the container and how to look up the values.



trunk/tests/test_cel.c
<https://reviewboard.asterisk.org/r/2646/#comment17759>

    I'd rename this to "verify_and_cleanup" or something like that



trunk/tests/test_cel.c
<https://reviewboard.asterisk.org/r/2646/#comment17758>

    Put a comment here explaining that the test CEL cleanup function also verifies the pass/fail status of each test.


- Matt Jordan


On June 26, 2013, 10:08 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2646/
> -----------------------------------------------------------
> 
> (Updated June 26, 2013, 10:08 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This adds several unit tests for CEL functionality and provides the requisite framework for creating additional unit tests.
> 
> This also cleans up some reference leaks that were occurring in Stasis-Core message callback code.
> 
> 
> Diffs
> -----
> 
>   trunk/include/asterisk/cel.h 393024 
>   trunk/main/cel.c 393024 
>   trunk/tests/test_cel.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/2646/diff/
> 
> 
> Testing
> -------
> 
> Ran the unit tests and ensured that the events logged were as expected.
> 
> 
> Thanks,
> 
> opticron
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130701/e2fce5ab/attachment-0001.htm>