Review Request 114605: Restore libkpeople unit tests

[Martin Klapetek]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/114605/#review46072
-----------------------------------------------------------

Ship it!


Good start. Couple remarks below...also not sure about the personpluginmanager.h method...


src/autotests/fakecontactsource.h
<https://git.reviewboard.kde.org/r/114605/#comment32871>

    Remove (all files)



src/autotests/fakecontactsource.h
<https://git.reviewboard.kde.org/r/114605/#comment32872>

    pointer signs



src/autotests/fakecontactsource.cpp
<https://git.reviewboard.kde.org/r/114605/#comment32873>

    Why this?



src/autotests/fakecontactsource.cpp
<https://git.reviewboard.kde.org/r/114605/#comment32874>

    Space



src/autotests/fakecontactsource.cpp
<https://git.reviewboard.kde.org/r/114605/#comment32875>

    Spaaaaace



src/autotests/persondatatests.cpp
<https://git.reviewboard.kde.org/r/114605/#comment32876>

    Could maybe possibly likely use better name - like changeContact1Email()? So it's clearer what it does.



src/personmanager.cpp
<https://git.reviewboard.kde.org/r/114605/#comment32868>

    Can you change the "QObject* parent" as well since you've already edited that line please



src/personmanager.cpp
<https://git.reviewboard.kde.org/r/114605/#comment32869>

    Should we hard-switch to QSQLITE?



src/personpluginmanager.h
<https://git.reviewboard.kde.org/r/114605/#comment32870>

    Yes it is.


- Martin Klapetek


On Dec. 22, 2013, 2:17 a.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114605/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2013, 2:17 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: libkpeople
> 
> 
> Description
> -------
> 
> Add tests for PersonData
> 
> 
> Add method to override DataSources used for testing
> 
> 
> Remove old nepomuk tests
> 
> 
> Add hook to alter the personsDB used
> 
> For making unit tests
> 
> 
> Remove unused include
> 
> 
> ---------------
> 
> I haven't tested all the things I want; I really want to test the model especially with lots of merging/unmerging
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 43e1008 
>   src/autotests/fakecontactsource.h PRE-CREATION 
>   src/autotests/fakecontactsource.cpp PRE-CREATION 
>   src/autotests/lib/CMakeLists.txt 2358e8b 
>   src/autotests/lib/NepomukTestLibMacros.cmake.in 5189462 
>   src/autotests/lib/datagenerator.h 6e75b67 
>   src/autotests/lib/datagenerator.cpp 630585a 
>   src/autotests/lib/nepomukserverrc.in 27b5c90 
>   src/autotests/lib/nepomuktest_export.h 38a8eaa 
>   src/autotests/lib/testbase.h 6d2a6b3 
>   src/autotests/lib/testbase.cpp 954fc83 
>   src/autotests/lib/tools/CMakeLists.txt 2e24962 
>   src/autotests/lib/tools/dbus-session-begin.sh ac0e63a 
>   src/autotests/lib/tools/dbus-session-end.sh 4a13a05 
>   src/autotests/lib/tools/nepomuk-sandbox-begin.sh.in a755590 
>   src/autotests/lib/tools/nepomuk-sandbox-end.sh.in e331cb0 
>   src/autotests/lib/tools/runNepomukTest.sh.in 18c73f9 
>   src/autotests/persondatatests.h PRE-CREATION 
>   src/autotests/persondatatests.cpp PRE-CREATION 
>   src/autotests/tests/CMakeLists.txt 63e6739 
>   src/autotests/tests/duplicatestests.h 2faed15 
>   src/autotests/tests/duplicatestests.cpp 4083825 
>   src/autotests/tests/persondatatests.h 90ec2a2 
>   src/autotests/tests/persondatatests.cpp bf0c470 
>   src/personmanager.cpp fd03ba7 
>   src/personmanager_p.h e334f53 
>   src/personpluginmanager.h adc7b9d 
>   src/personpluginmanager.cpp e433bb5 
> 
> Diff: https://git.reviewboard.kde.org/r/114605/diff/
> 
> 
> Testing
> -------
> 
> Tested my new tests!
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20131223/fe01b966/attachment-0001.html>