Review Request 122206: [kio] Make tests optional

[Ben Cooksley]

> On March 17, 2015, 3:37 a.m., Albert Vaca Cintora wrote:
> > I know this is merged already but this patch is being applied to every KDE package and I want to keep the discussion in a single place.
> > 
> > We already have a toggle option in CMake that is "BUILD_TESTING". If Gentoo wants to not build the tests (I'm not judging if they should, let them be free to do it), they can just set BUILD_TESTING to OFF. I understand that CMake will still try to find Qt5Test and fail, but here is where I think we got it wrong:
> > 
> > This patch does the following:
> > if (Qt5Test is not found) 
> >     BUILD_TESTING = OFF
> > 
> > What I think this patch should be doing is this:
> > if (BUILD_TESTING == OFF) 
> >     Don't look for Qt5Test
> > 
> > Did I miss something or this seems more reasonable to you guys as well?
> 
> Michael Palimaka wrote:
>     One of the original versions of these test patches looked something like:
>     
>         if (BUILD_TESTING)
>             add_subdirectory(autotests)
>         endif ()
>         
>     with `find_package(Qt5Test)` inside the autotests directory. While this is used a bit in frameworks, it was rejected from a lot of plasma packages because it relies on a magic variable (although it is defined by ECM).
>     
>     As a result there's a whole range of approaches across frameworks/plasma/apps all doing the same thing. It would be nice if we could agree on something and be consistent.
> 
> Albert Vaca Cintora wrote:
>     Is that the reason why we are doing it this way? :/ I don't think this is a magic variable at all, and if you want it to be even less magic you can set it in advance in a line before the if.
> 
> Albert Astals Cid wrote:
>     I don't like any of the patches, but i prefer Albert's suggestion way over the automagic disabling of the tests. If you don't want tests, just manually specify it.

As long as the variable name is consistent across all packages, and specifying it to on forces Qt5Test to be searched for i'm happy.
Albert's above proposal therefore makes more sense from a "less magic" point of view.

Any package which deviates from the consistent name (at this time: BUILD_TESTING) won't have test coverage on the CI system.


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122206/#review77603
-----------------------------------------------------------


On Feb. 7, 2015, 12:14 a.m., Andreas Sturmlechner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122206/
> -----------------------------------------------------------
> 
> (Updated Feb. 7, 2015, 12:14 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> [kio] Make tests optional
> This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c1ed03f6cac648517828aec60e896baf9fbcfd9d 
> 
> Diff: https://git.reviewboard.kde.org/r/122206/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andreas Sturmlechner
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150318/aa52ee84/attachment.html>