--===============1180897941== Content-Type: multipart/alternative; boundary="===============1922012565286809129==" --===============1922012565286809129== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101790/#review4437 ----------------------------------------------------------- nepomuk/services/strigi/strigiservice.h I would prefer a method which is also exposed via DBus. This makes sens= e in any case. I would often like to change the indexing speed for manual t= esting for example. = But for that I would not use the IndexScheduler enum but an int which d= irectly 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 Please follow the code indentation policy in KDE. Methods do not use tr= ailing brackets. nepomuk/tests/indexertests.cpp KConfig objects are typically created on the stack. The only "advantage= " of this approach is a nice memory leak. ;) nepomuk/tests/indexertests.cpp Is QDir::currentPath() really appropriate? The test could be started fr= om anywhere? How about a cmale config files which puts the actual path to t= he test data into the source file? nepomuk/tests/indexertests.cpp "NULL" is a C definition. Please use "0" instead. nepomuk/tests/indexertests.cpp Superfluous variable. nepomuk/tests/indexertests.cpp #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 Hm, how about we add a command line parameter to nepomukserver which sp= ecifies which services to start. Otherwise new services not mentioned in th= is list will be started. nepomuk/tests/lib/testbase.cpp This is never deleted. nepomuk/tests/lib/testbase.cpp See comment about the nepomukserver parameter. Once we have that there = is no need to stop services here. nepomuk/tests/lib/testbase.cpp This is IMHO unclean. It would be best to simply stop Virtuoso, recreat= e 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 KIO can do that for you. nepomuk/tests/test/filewatch/filewatch.cpp Tabs are evil. :P nepomuk/tests/test/filewatch/filewatch.cpp Again KIO can do this. nepomuk/tests/test/filewatch/filewatch.cpp This is very ugly. How about we add a signal to the filewatcher DBus in= terface which is emitted once the filewatcher has handled its queue. nepomuk/tests/test/filewatch/filewatch.cpp Use KTempDir instead. - Sebastian On June 28, 2011, 7:31 a.m., Pawe=C5=82 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 te= st 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.cp= p). > = > 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 pro= blems (segfaults) with sendEvents method that notifies the user that "index= ing started for fast search" - I had to disable it locally, I will try to t= rack down the cause of this if it is reproducible. > = > = > Thanks, > = > Pawe=C5=82 > = > --===============1922012565286809129== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/101790/

= =
nepomuk/services/strigi/strigiservice.h (Diff revision 2) 


  

 

  

    
    

    45
    
        		IndexScheduler::I=
ndexingSpeed speed =3D IndexScheduler::SnailPace );

  


 





I would prefer a method which is also exposed via DBus. This makes s=
ense in any case. I would often like to change the indexing speed for manua=
l testing for example.

But for that I would not use the IndexScheduler enum but an int which direc=
tly expresses the delay between files.

Then we could even change the enum to have msec values or even replace it b=
y static const ints.

















 =

 =




 

 
  

   
    nepomuk/tests/indexertests.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
using namespace Nepomuk;


  

 

  

    
    

    50
    
void IndexerTests::testSimp=
leIndexing() {

  


 





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

















 =

 =




 

 
  

   
    nepomuk/tests/indexertests.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
using namespace Nepomuk;


  

 

  

    
    

    82
    
	KConfig* config =3D new KConfig("nepomukstrigirc" );

  


 





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

















 =

 =




 

 
  

   
    nepomuk/tests/indexertests.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
using namespace Nepomuk;


  

 

  

    
    

    84
    
	config->group("General").writePathEntry("folders", QDi=
r::currentPath().append(<=
/span>"/testdata"));

  


 





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

















 =

 =




 

 
  

   
    nepomuk/tests/indexertests.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
using namespace Nepomuk;


  

 

  

    
    

    87
    
	Nepomuk::StrigiService s<=
/span>(NULL, QList<=
QVariant>(), IndexScheduler=
::FullSpeed); // This should start the strigi service and index scheduler.=


  


 





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

















 =

 =




 

 
  

   
    nepomuk/tests/indexertests.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
using namespace Nepomuk;


  

 

  

    
    

    89
    
	bool gotSignal =3D QTest::kWaitForSignal(&s, SIGNAL(indexingStopped()),<=
/span> 15000);

  


 





Superfluous variable.

















 =

 =




 

 
  

   
    nepomuk/tests/indexertests.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
using namespace Nepomuk;


  

 

  

    
    

    110
    
QTEST_KDEMAIN(IndexerTests,<=
/span> NoGUI)

  


 





#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
    

     (Diff revision 2)

    
   
  

 

 
  


   


  

 

  

    
    

    1
    
[Basic Settings]

  


 





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
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
Nepomuk::TestBase::TestBase(QObject* parent)


  

 

  

    
    

    53
    
    d->m_serviceManager =
=3D new org::kde::=
nepomuk::ServiceManager( "o=
rg.kde.NepomukServer", "/servicemanager", QDBusConnection::s=
essionBus() );

  


 





This is never deleted.

















 =

 =




 

 
  

   
    nepomuk/tests/lib/testbase.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
Nepomuk::TestBase::TestBase(QObject* parent)


  

 

  

    
    

    64
    
    Q_FOREACH( const QString=
 & service, services =
)

  


 





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

















 =

 =




 

 
  

   
    nepomuk/tests/lib/testbase.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
Nepomuk::TestBase::TestBase(QObject* parent)


  

 

  

    
    

    84
    
{

  


 





This is IMHO unclean. It would be best to simply stop Virtuoso, recr=
eate the DB from the tar and restart Virtuoso.

The removeAllStatements is not really public API anyway anymore and the sto=
rage service does create some information we might miss here.

This, however, has a low priority for me. :)

















 =

 =




 

 
  

   
    nepomuk/tests/test/filewatch/filewatch.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
namespace {


  

 

  

    
    

    51
    
    // It's hard to bel=
ieve that Qt doesn't provide its own copy folder function
<=
/td>
  


 





KIO can do that for you.

















 =

 =




 

 
  

   
    nepomuk/tests/test/filewatch/filewatch.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
namespace {


  

 

  

    
    

    54
    
        

  


 





Tabs are evil. :P

















 =

 =




 

 
  

   
    nepomuk/tests/test/filewatch/filewatch.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
namespace {


  

 

  

    
    

    84
    
    bool removeDir(const QString &dirName)

  


 





Again KIO can do this.

















 =

 =




 

 
  

   
    nepomuk/tests/test/filewatch/filewatch.cpp
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
namespace {


  

 

  

    
    

    134
    
    QTest::qSleep( <=
span class=3D"mi">4000 );

  


 





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
    

     (Diff revision 2)

    
   
  

 

 
  


   

   
namespace {


  

 

  

    
    

    176
    
QString FileWatchTest::cre=
ateTempDir()

  


 





Use KTempDir instead.









- Sebastian






On June 28th, 2011, 7:31 a.m., Pawe=C5=82 Paprota wrote:









 

  


Review request for Nepomuk.


By Pawe=C5=82 Paprota.




Updated June 28, 2011, 7:31 a.m.






Descripti=
on 



 

  
   
This is a little ground work for writing integration tests i=
n order to test Nepomuk components in the "real" setup - with D-B=
us/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

  		
 






Testing <=
/h1>


 

  
   
Tests should execute with simple "make test". I di=
d however have some problems (segfaults) with sendEvents method that notifi=
es the user that "indexing started for fast search" - I had to di=
sable it locally, I will try to track down the cause of this if it is repro=
ducible.

  		
 








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 (2=
eb7eac)
    • 

 
  • 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-CREATI=
ON)
    • 

 
  • nepomuk/tests/lib/CMakeLists.txt (PRE-CREA=
TION)
    • 

 
  • 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 (PR=
E-CREATION)
    • 

 
  • nepomuk/tests/lib/testbase.h (PRE-CREATION=
)
    • 

 
  • nepomuk/tests/lib/testbase.cpp (PRE-CREATI=
ON)
    • 

 
  • nepomuk/tests/runNepomukTest.sh (PRE-CREAT=
ION)
    • 

 
  • nepomuk/tests/test/CMakeLists.txt (PRE-CRE=
ATION)
    • 

 
  • nepomuk/tests/test/filewatch/CMakeLists.txt (=
PRE-CREATION)
    • 

 
  • nepomuk/tests/test/filewatch/filewatch.cpp (PRE-CREATION)
    • 

 
  • nepomuk/tests/test/identificationtest.h (P=
RE-CREATION)
    • 

 
  • nepomuk/tests/test/identificationtest.cpp =
(PRE-CREATION)
    • 

 
  • nepomuk/tests/testdata/CMakeLists.txt (PRE=
-CREATION)
    • 

 
  • nepomuk/tests/testdata/klogo.png (PRE-CREA=
TION)
    • 

 
  • nepomuk/tests/testdata/trollface.jpg (PRE-=
CREATION)
    • 





View Diff





  		
 











  

 



--===============1922012565286809129==--

--===============1180897941==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Nepomuk mailing list
Nepomuk@kde.org
https://mail.kde.org/mailman/listinfo/nepomuk

--===============1180897941==--