[Nepomuk] Re: Review Request: Setup for integration tests in Nepomuk + a few indexer tests

Wed Jul 6 11:00:44 CEST 2011

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


nepomuk/services/strigi/strigiservice.h
<http://git.reviewboard.kde.org/r/101790/#comment3726>

    I would prefer a method which is also exposed via DBus. This makes sense in any case. I would often like to change the indexing speed for manual testing for example.
    
    But for that I would not use the IndexScheduler enum but an int which directly expresses the delay between files.
    
    Then we could even change the enum to have msec values or even replace it by static const ints.


nepomuk/tests/indexertests.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3727>

    Please follow the code indentation policy in KDE. Methods do not use trailing brackets.


nepomuk/tests/indexertests.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3728>

    KConfig objects are typically created on the stack. The only "advantage" of this approach is a nice memory leak. ;)


nepomuk/tests/indexertests.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3732>

    Is QDir::currentPath() really appropriate? The test could be started from anywhere? How about a cmale config files which puts the actual path to the test data into the source file?


nepomuk/tests/indexertests.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3729>

    "NULL" is a C definition. Please use "0" instead.


nepomuk/tests/indexertests.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3730>

    Superfluous variable.


nepomuk/tests/indexertests.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3731>

    #include "indexertests.moc"
    
    This is just policy and slightly improves compilation performance since the moc file does not have to be compiled independently.


nepomuk/tests/lib/nepomukserverrc.in
<http://git.reviewboard.kde.org/r/101790/#comment3733>

    Hm, how about we add a command line parameter to nepomukserver which specifies which services to start. Otherwise new services not mentioned in this list will be started.


nepomuk/tests/lib/testbase.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3734>

    This is never deleted.


nepomuk/tests/lib/testbase.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3735>

    See comment about the nepomukserver parameter. Once we have that there is no need to stop services here.


nepomuk/tests/lib/testbase.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3736>

    This is IMHO unclean. It would be best to simply stop Virtuoso, recreate the DB from the tar and restart Virtuoso.
    
    The removeAllStatements is not really public API anyway anymore and the storage service does create some information we might miss here.
    
    This, however, has a low priority for me. :)


nepomuk/tests/test/filewatch/filewatch.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3738>

    KIO can do that for you.


nepomuk/tests/test/filewatch/filewatch.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3737>

    Tabs are evil. :P


nepomuk/tests/test/filewatch/filewatch.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3739>

    Again KIO can do this.


nepomuk/tests/test/filewatch/filewatch.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3741>

    This is very ugly. How about we add a signal to the filewatcher DBus interface which is emitted once the filewatcher has handled its queue.


nepomuk/tests/test/filewatch/filewatch.cpp
<http://git.reviewboard.kde.org/r/101790/#comment3740>

    Use KTempDir instead.


- Sebastian


On June 28, 2011, 7:31 a.m., Paweł Paprota wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101790/
> -----------------------------------------------------------
> 
> (Updated June 28, 2011, 7:31 a.m.)
> 
> 
> Review request for Nepomuk.
> 
> 
> Summary
> -------
> 
> This is a little ground work for writing integration tests in order to test Nepomuk components in the "real" setup - with D-Bus/socket communication, Virtuoso repository, KConfig configuration running etc.
> 
> Also included are simple tests for indexing of files (see indexertests.cpp).
> 
> See README for details.
> 
> The code is also available here:
> 
> https://github.com/ppawel/kde-runtime/tree/integration-tests
> 
> 
> Diffs
> -----
> 
>   nepomuk/CMakeLists.txt c6a1879 
>   nepomuk/services/strigi/indexscheduler.cpp 56914a7 
>   nepomuk/services/strigi/nepomukindexer.h 08abb2a 
>   nepomuk/services/strigi/nepomukindexer.cpp d796983 
>   nepomuk/services/strigi/strigiservice.h 2eb7eac 
>   nepomuk/services/strigi/strigiservice.cpp 0189679 
>   nepomuk/tests/CMakeLists.txt PRE-CREATION 
>   nepomuk/tests/README PRE-CREATION 
>   nepomuk/tests/indexertests.h PRE-CREATION 
>   nepomuk/tests/indexertests.cpp PRE-CREATION 
>   nepomuk/tests/lib/CMakeLists.txt PRE-CREATION 
>   nepomuk/tests/lib/NepomukTestLibMacros.cmake PRE-CREATION 
>   nepomuk/tests/lib/nepomuk-repository-with-ontologies.tgz PRE-CREATION 
>   nepomuk/tests/lib/nepomukserverrc.in PRE-CREATION 
>   nepomuk/tests/lib/nepomuktest_export.h PRE-CREATION 
>   nepomuk/tests/lib/testbase.h PRE-CREATION 
>   nepomuk/tests/lib/testbase.cpp PRE-CREATION 
>   nepomuk/tests/runNepomukTest.sh PRE-CREATION 
>   nepomuk/tests/test/CMakeLists.txt PRE-CREATION 
>   nepomuk/tests/test/filewatch/CMakeLists.txt PRE-CREATION 
>   nepomuk/tests/test/filewatch/filewatch.h PRE-CREATION 
>   nepomuk/tests/test/filewatch/filewatch.cpp PRE-CREATION 
>   nepomuk/tests/test/identificationtest.h PRE-CREATION 
>   nepomuk/tests/test/identificationtest.cpp PRE-CREATION 
>   nepomuk/tests/testdata/CMakeLists.txt PRE-CREATION 
>   nepomuk/tests/testdata/klogo.png PRE-CREATION 
>   nepomuk/tests/testdata/trollface.jpg PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/101790/diff
> 
> 
> Testing
> -------
> 
> Tests should execute with simple "make test". I did however have some problems (segfaults) with sendEvents method that notifies the user that "indexing started for fast search" - I had to disable it locally, I will try to track down the cause of this if it is reproducible.
> 
> 
> Thanks,
> 
> Paweł
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/nepomuk/attachments/20110706/fbbb0fb4/attachment-0001.htm