D7930: Kdevelop CMake plugin : use canonical paths to build.dir

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  No, it should not just be a test around `QFileInfo::canonicalFilePath`, that is done already in Qt upstream for us. What I want is test(s) for the CMakeManager code you are changing. I.e. take `CMakeManager::buildDirectory` as an example. Extend the `test_cmakemanager` (i.e. add another test method there). Then play with symlink structure in a `QTemporaryDir` to replicate your erroneous setup. Finally call `CMakeManager::buildDirectory` and ensure it returns "the right thing". Similarly you can call the cmakeutils functions and check their return values for sanity.
  
  This test should fail before you apply this patch, and then pass with this patch.

INLINE COMMENTS

> cmakeutils.h:95
> +     */
> +    KDEVCMAKECOMMON_EXPORT KDevelop::Path currentCanonicalBuildDir( KDevelop::IProject* project, int builddir = -1 );
> +

When should this function be used instead of the above? Shouldn't the above always return the canonical version?

> cmakeutils.h:202
> +     */
> +    KDEVCMAKECOMMON_EXPORT QStringList allCanonicalBuildDirs(KDevelop::IProject* project);
> +

same as above

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D7930

To: rjvbb, #kdevelop, apol, mwolff
Cc: flherne, mwolff, apol, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171019/da35f7d7/attachment.html>