bruns requested changes to this revision. bruns added a comment. This revision now requires changes to proceed.
The unit test lacks structure, please think about: - how you can split this into separate test cases (basic functionality, renaming, ...) - how to use a data driven approach for the test, i.e. splitting the basic test into one entry per file INLINE COMMENTS > unindexedfileiteratortest.cpp:58 > { > - // Bah!! > - // Testing this is complex! > - // FIXME: How in the world should I test this? > + QStringList dirs; > + dirs << QStringLiteral("home/"); Bad variable name, these are files **and** dirs. > unindexedfileiteratortest.cpp:62 > + dirs << QStringLiteral("home/2"); > + dirs << QStringLiteral("home/kde/"); > + dirs << QStringLiteral("home/kde/1"); bad name, use something like excluded. > unindexedfileiteratortest.cpp:98 > + QString indexedFile("home/docs/1"); > + QString mimeType = m_mimeDb.mimeTypeForFile(dirPrefix + indexedFile, > QMimeDatabase::MatchExtension).name(); > + BasicIndexingJob job(dirPrefix + indexedFile, mimeType, > BasicIndexingJob::NoLevel); you don't actually have to determine the mimetype here, just use "text/plain" > unindexedfileiterator.cpp:126 > << timeInfo.cTime << fileMTime; > + // Since documentUrl is pretty expensive, we want to calculate it > only > + // if we suspect it could have changed This whole block belongs into a separate block, executed `if (m_cTimeChanged)`. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D21509 To: poboiko, #frameworks, #baloo, bruns Cc: kde-frameworks-devel, gennad, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams