Review Request: Tests for gluonobject

[Laszlo Papp]

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



core/tests/gluonobjecttest.h
<http://git.reviewboard.kde.org/r/102483/#comment5488>

    testQualifiedName



core/tests/gluonobjecttest.h
<http://git.reviewboard.kde.org/r/102483/#comment5489>

    testFullyQualifiedName



core/tests/gluonobjecttest.h
<http://git.reviewboard.kde.org/r/102483/#comment5490>

    testSetPropertyData



core/tests/gluonobjecttest.cpp
<http://git.reviewboard.kde.org/r/102483/#comment5495>

    I would probably make a childObjectName string and use that inside the test method. It would seem less error prone.



core/tests/gluonobjecttest.cpp
<http://git.reviewboard.kde.org/r/102483/#comment5491>

    testQualifiedName



core/tests/gluonobjecttest.cpp
<http://git.reviewboard.kde.org/r/102483/#comment5496>

    This is a very good example for the gluonObject*Name case. It is very handy and nice here to avoid the error-prone situation by typos and so forth, if you have dedicated strings.



core/tests/gluonobjecttest.cpp
<http://git.reviewboard.kde.org/r/102483/#comment5492>

    testFullyQualifiedName



core/tests/gluonobjecttest.cpp
<http://git.reviewboard.kde.org/r/102483/#comment5497>

    Dedicated const QString variables.



core/tests/gluonobjecttest.cpp
<http://git.reviewboard.kde.org/r/102483/#comment5494>

    Could you please build the string before the comparison ? It would be more readable, and you could also uses space after the "+" characters.



core/tests/gluonobjecttest.cpp
<http://git.reviewboard.kde.org/r/102483/#comment5493>

    I would probably make a hash (or map) for the Int/int(-1) and so forth string values. Afterwards, I would write just one QTest::newRow with a loop. That way, the typo could be simplified very radically. 
    
    Some people do not like having too much logic in test code, but I think a foreach loop is not that high risk, nor too much logic. :)


- Laszlo


On Sept. 1, 2011, 6:29 p.m., Ashwin Rajeev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102483/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2011, 6:29 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Summary
> -------
> 
> Add few new test cases in gluonobjecttest
> {testAddChildAt,testQualifiedname,testFullyqualifiedname,testNameToObjectName,testSetProperty}
> 
> 
> Diffs
> -----
> 
>   core/tests/gluonobjecttest.h a9e0d06 
>   core/tests/gluonobjecttest.cpp 5b380a6 
> 
> Diff: http://git.reviewboard.kde.org/r/102483/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashwin
> 
>

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