Review Request: Tests for GDLHandler

[Laszlo Papp]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102511/#review6315
-----------------------------------------------------------



core/tests/gdlhandlertest.h
<http://git.reviewboard.kde.org/r/102511/#comment5565>

    I think x, e and r could also be more talkative :)
    
    I removed the static modifier since that is probably another left-over. Also, the definition was put into the source file. Sync up please (=rebase).



core/tests/gdlhandlertest.h
<http://git.reviewboard.kde.org/r/102511/#comment5567>

    Please remove the static modified and put the code into the source file.



core/tests/gdlhandlertest.h
<http://git.reviewboard.kde.org/r/102511/#comment5566>

    'r' could also be a more talkative name here instead.



core/tests/gdlhandlertest.h
<http://git.reviewboard.kde.org/r/102511/#comment5576>

    Please use imperative like few lines above, so "Return".



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5568>

    It is not the fruit of the current patch, but please use more talkative names than these. On the other hand: a_parent does not really fit to the camelCase style.



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5569>

    You might need to change this line accordingly.



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5575>

    1) I would personally prefer Q_ASSERT_X in test methods to make the issue more clear with a descriptive message
    
    2)However I think this line is not needed after you check those lists have the same size, thus it could just be removed.



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5570>

    Could you please use the ';' on the last line where there is a string character ? We do not normally put ';' into separate lines.



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5571>

    Same here, ';' could be one line above.



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5572>

    Same here, ';' could be one line above.



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5573>

    Same here, ';' could be one line above.



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5574>

    Could you please use true here as well ?


- Laszlo


On Sept. 6, 2011, 6:13 p.m., Ashwin Rajeev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102511/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2011, 6:13 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Summary
> -------
> 
> Add few new test cases in gdlhandlertest
> {ensureParsing,ensureSerializing,ensureCommenting,testGDLSample,testCommentAtBegin,testCommentAtEnd}
>     
> Modify compareTrees method to check children of gluonobjects
> 
> 
> Diffs
> -----
> 
>   core/tests/gdlhandlertest.h 8302491 
>   core/tests/gdlhandlertest.cpp 90ac380 
> 
> Diff: http://git.reviewboard.kde.org/r/102511/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashwin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/gluon/attachments/20110906/af8cf397/attachment-0001.html>