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
