D21509: [baloo_file] Index renamed folders inside UnindexedFileIndexer

Stefan Brüns noreply at phabricator.kde.org
Mon Jun 3 00:41:30 BST 2019


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190602/82a4eebe/attachment.html>


More information about the Kde-frameworks-devel mailing list