D19673: Discovered unit-tests using bracket arguments and/or listed in files other than CTestTestFile.cmake

[Milian Wolff]

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


  great! can we get a unit test for this? though I'm unsure if we ever revived the unit tests from our old cmake integration
  
  I've some small style nits, but otherwise +1

INLINE COMMENTS

> cmakeutils.cpp:694
>  
> -QVector<Test> importTestSuites(const Path &buildDir)
> +QVector<Test> importTestSuites(const Path &buildDir, const QString & CMakeTestFileName)
>  {

then add this function here (with the style fixes mentioned above), and have importTestSuites defer to it via

  QVector<Test> importTestSuites(const Path &buildDir)
  {
      return importTestSuites(buildDir, QStringLiteral("CTestTestfile.cmake"));
  }

> cmakeutils.cpp:696
>  {
> -    const auto contents = CMakeListsParser::readCMakeFile(buildDir.toLocalFile() + QLatin1String("/CTestTestfile.cmake"));
> -
> +    auto CMakeTestFile = Path(buildDir, CMakeTestFileName).toLocalFile()  ;
> +    const auto contents = CMakeListsParser::readCMakeFile(CMakeTestFile);

const and lower-case the start

> cmakeutils.cpp:714
> +            // Include directive points directly to a .cmake file hosting the tests
> +            tests += importTestSuites(Path(buildDir, entry.arguments.first().value), QStringLiteral(""));
>          } else if (entry.name == QLatin1String("set_tests_properties")) {

`QString()` or `{}` instead of empty string literal

> cmakeutils.h:265
>  
> -    KDEVCMAKECOMMON_EXPORT QVector<Test> importTestSuites(const KDevelop::Path &buildDir);
> +    KDEVCMAKECOMMON_EXPORT QVector<Test> importTestSuites(const KDevelop::Path &buildDir, const QString & CMakeTestFileName=QStringLiteral("CTestTestfile.cmake"));
>  }

remove space after &, but add spaces around = and lower-case the cmakeTestFileName

looking below, it would probably be better to split this up, i.e. only keep the importTestSuites public that takes only a Path (i.e. the old API)

REPOSITORY
  R32 KDevelop

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

To: tnorth, apol, kfunk, mwolff
Cc: mwolff, kdevelop-devel, gennad, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190311/0758b138/attachment.html>