[asterisk-dev] [Code Review] astobj2 unit test

[Russell Bryant]

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



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/496/#comment3347>

    Every time you add trailing whitespace, a kitten loses its meow.



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/496/#comment3350>

    Why keep running the test at this point?  It's going to crash, probably.



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/496/#comment3351>

    Please use snprintf().  This will generate a compiler warning on picky systems.



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/496/#comment3353>

    Are you actually going to get anything here without passing an object to compare against?



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/496/#comment3354>

    This looks wrong.  I don't understand the "remove ref from link" line.



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/496/#comment3355>

    What does this loop test?  Just make sure it doesn't crash?



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/496/#comment3356>

    Another interesting test you could do at this point is an ao2_callback() on the whole container to make sure the ref count on every object is 1.  ao2_ref(obj, 0) gives you the current ref count.



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/496/#comment3357>

    It could be that the destructor was called too many times, as well



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/496/#comment3349>

    Is there a more descriptive name we could use?



/trunk/tests/test_astobj2.c
<https://reviewboard.asterisk.org/r/496/#comment3348>

    Does the lim variable provide any benefit?  Perhaps you were going to include in some status messages?


- Russell


On 2010-02-09 14:44:54, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/496/
> -----------------------------------------------------------
> 
> (Updated 2010-02-09 14:44:54)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This is a unit test for main/astobj2.c.  All the status update calls will be changed to match Russell's tweak once it is committed.  That is why I do not set the error string anywhere.
> 
> 
> Diffs
> -----
> 
>   /trunk/tests/test_astobj2.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/496/diff
> 
> 
> Testing
> -------
> 
> ran unit test, verified it passes. 
> 
> 
> Thanks,
> 
> David
> 
>