Re: Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated April 22, 2017, 10:35 p.m.) Status -- This change has been discarded. 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
Re: Review Request 122206: [kio] Make tests optional
On March 17, 2015, 4: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. Ben Cooksley wrote: 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 why? It is enabled by default anyway. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review77603 --- On Feb. 7, 2015, 1: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, 1: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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
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. Ben Cooksley wrote: 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. Aleix Pol Gonzalez wrote: @Ben why? It is enabled by default anyway. @Aleix: What I mean is that we won't be accepting requests to have the CI system pass special variables as part of the CMake invocation in the event a project does decide to use a different variable (without good reason). Projects can default BUILD_TESTING to On or Off, but the CI system will always pass a flag setting it to On. - 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On March 16, 2015, 8:37 p.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. 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 --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review77603 --- On Feb. 6, 2015, 4:14 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Feb. 6, 2015, 4:14 p.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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On mar. 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. 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. - Albert --- 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
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? 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. - Michael --- 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review77603 --- 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? - Albert Vaca Cintora On Feb. 6, 2015, 4:14 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Feb. 6, 2015, 4:14 p.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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review75568 --- Ship it! Ship It! - David Faure 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 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
--- 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. Changes --- Do it like kwin. Repository: kio Description --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs (updated) - CMakeLists.txt c1ed03f6cac648517828aec60e896baf9fbcfd9d Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Feb. 1, 2015, 2:33 p.m., David Faure wrote: Let me get this straight. This patch makes the option BUILD_TESTING work, i.e. skip testing if not set. The solution that was committed to kwin, *additionally* sets BUILD_TESTING to false if Qt5Test isn't available. This doesn't prevent setting the option to false manually if someone has Qt5Test but doesn't want to build the tests. Is this an accurate description of the issue? I would add that, IMHO, all frameworks should handle this the same way. I had a further look and they currently don't. * kservice, kimageformats, ktexteditor... do it like this patch (manual option). * kcoreaddons, karchive... don't have the option, they just skip autotests if Qt5Test is not found. * ki18n, kguiaddons, kdbusaddons, threadweaver compile autotests unconditionally. I'm confused - how did anyone without Qt5Test make it all the way to kio, which depends on kdbusaddons and ki18n, then? Unless I missed something, I would say that by default tests should be compiled if Qt5Test is present (developers) and should not be compiled if Qt5Test is absent or if it is explicitly desired not to build tests (packagers). Therefore: 1) doing both steps is good (i.e. doing like kwin). 2) it should be done in all frameworks (that have autotests, which is most of them) Anyone up to the task? I guess what I was really trying to do initially - besides changing as little as possible - was mimicking the existing behaviour as much as possible. That means bailing out when Qt5Test is not found even though BUILD_TESTING was requested, so the user is informed about a dependency problem. The kwin solution will silently not build the tests, as far as I see it. If this is fine from a developer/packager point of view, I have a new patch for kio ready that goes the kwin way. @David, why it was discovered in kio, that's easy, you do some packaging for fun and get told your submission should not hard-depend on Qt5Test, then you uninstall that to look for any fallout and kio happens to be one of the next few packages to receive new commits, thus rebuild fails (because the existing test-removing automagic did not work). - Andreas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review75140 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:58 p.m., David Faure wrote: Not sure why this is suddenly triggering many philosophical discussions about what users should be doing (IMHO, give them choice, this is opensource). Similar changes have been done in most other frameworks long ago, this is most certainly consistent. (see e.g. kcoreaddons - the find_package(Qt5Test) is being done in the autotests subfolder, the same could be done here). Vishesh Handa wrote: It's not about users, it's about developers/packagers. We're not forcing them to run the tests, we're telling them to compile them. They always have a choice by modifying the code, we as upstream shouldn't be making it easier for them to skip the most of basic of quality tests. Martin Gräßlin wrote: @Vishesh: as I went through the same thing I point you to https://git.reviewboard.kde.org/r/117393/ - it was a discussion for the same on KWin and now I think it was good that we accepted the solution in the end. Maybe the reasoning provided there helps to understand the packagers needs. Vishesh Handa wrote: @Martin: I do prefer the solution in the kwin patch. This one is really messy. Though I still don't agree with their reasons. @David, I should add that I can not commit this myself, without write access, so I ask for someone to do it. Unless I should come up with a solution similar to Martin's, so more people could agree? - Andreas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74633 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review75140 --- Let me get this straight. This patch makes the option BUILD_TESTING work, i.e. skip testing if not set. The solution that was committed to kwin, *additionally* sets BUILD_TESTING to false if Qt5Test isn't available. This doesn't prevent setting the option to false manually if someone has Qt5Test but doesn't want to build the tests. Is this an accurate description of the issue? I would add that, IMHO, all frameworks should handle this the same way. I had a further look and they currently don't. * kservice, kimageformats, ktexteditor... do it like this patch (manual option). * kcoreaddons, karchive... don't have the option, they just skip autotests if Qt5Test is not found. * ki18n, kguiaddons, kdbusaddons, threadweaver compile autotests unconditionally. I'm confused - how did anyone without Qt5Test make it all the way to kio, which depends on kdbusaddons and ki18n, then? Unless I missed something, I would say that by default tests should be compiled if Qt5Test is present (developers) and should not be compiled if Qt5Test is absent or if it is explicitly desired not to build tests (packagers). Therefore: 1) doing both steps is good (i.e. doing like kwin). 2) it should be done in all frameworks (that have autotests, which is most of them) Anyone up to the task? - David Faure On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 9:58 p.m., David Faure wrote: Not sure why this is suddenly triggering many philosophical discussions about what users should be doing (IMHO, give them choice, this is opensource). Similar changes have been done in most other frameworks long ago, this is most certainly consistent. (see e.g. kcoreaddons - the find_package(Qt5Test) is being done in the autotests subfolder, the same could be done here). Vishesh Handa wrote: It's not about users, it's about developers/packagers. We're not forcing them to run the tests, we're telling them to compile them. They always have a choice by modifying the code, we as upstream shouldn't be making it easier for them to skip the most of basic of quality tests. Martin Gräßlin wrote: @Vishesh: as I went through the same thing I point you to https://git.reviewboard.kde.org/r/117393/ - it was a discussion for the same on KWin and now I think it was good that we accepted the solution in the end. Maybe the reasoning provided there helps to understand the packagers needs. Vishesh Handa wrote: @Martin: I do prefer the solution in the kwin patch. This one is really messy. Though I still don't agree with their reasons. Andreas Sturmlechner wrote: @David, I should add that I can not commit this myself, without write access, so I ask for someone to do it. Unless I should come up with a solution similar to Martin's, so more people could agree? @Andreas: I think the solution in KWin is better and more straight forward for both packagers and developers. So I recommend to change to such an approach. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74633 --- On Jan. 22, 2015, 8:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 8:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 9:58 p.m., David Faure wrote: Not sure why this is suddenly triggering many philosophical discussions about what users should be doing (IMHO, give them choice, this is opensource). Similar changes have been done in most other frameworks long ago, this is most certainly consistent. (see e.g. kcoreaddons - the find_package(Qt5Test) is being done in the autotests subfolder, the same could be done here). Vishesh Handa wrote: It's not about users, it's about developers/packagers. We're not forcing them to run the tests, we're telling them to compile them. They always have a choice by modifying the code, we as upstream shouldn't be making it easier for them to skip the most of basic of quality tests. @Vishesh: as I went through the same thing I point you to https://git.reviewboard.kde.org/r/117393/ - it was a discussion for the same on KWin and now I think it was good that we accepted the solution in the end. Maybe the reasoning provided there helps to understand the packagers needs. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74633 --- On Jan. 22, 2015, 8:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 8:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:58 p.m., David Faure wrote: Not sure why this is suddenly triggering many philosophical discussions about what users should be doing (IMHO, give them choice, this is opensource). Similar changes have been done in most other frameworks long ago, this is most certainly consistent. (see e.g. kcoreaddons - the find_package(Qt5Test) is being done in the autotests subfolder, the same could be done here). Vishesh Handa wrote: It's not about users, it's about developers/packagers. We're not forcing them to run the tests, we're telling them to compile them. They always have a choice by modifying the code, we as upstream shouldn't be making it easier for them to skip the most of basic of quality tests. Martin Gräßlin wrote: @Vishesh: as I went through the same thing I point you to https://git.reviewboard.kde.org/r/117393/ - it was a discussion for the same on KWin and now I think it was good that we accepted the solution in the end. Maybe the reasoning provided there helps to understand the packagers needs. @Martin: I do prefer the solution in the kwin patch. This one is really messy. Though I still don't agree with their reasons. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74633 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:58 p.m., David Faure wrote: Not sure why this is suddenly triggering many philosophical discussions about what users should be doing (IMHO, give them choice, this is opensource). Similar changes have been done in most other frameworks long ago, this is most certainly consistent. (see e.g. kcoreaddons - the find_package(Qt5Test) is being done in the autotests subfolder, the same could be done here). It's not about users, it's about developers/packagers. We're not forcing them to run the tests, we're telling them to compile them. They always have a choice by modifying the code, we as upstream shouldn't be making it easier for them to skip the most of basic of quality tests. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74633 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 1:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. Matthew Dawson wrote: At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - If the user doesn't want to run them, I'm sure Gentoo can provide some options for that. Compiling them cannot be such a huge cost. - They are already burning CPU time by using a source based distro. This way they might actually catch some bugs and possibly not waste developers time by filing bugs which may have been an issue with their system. I'm not sure if I will approve such patches on packages I maintain. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:43 a.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 2:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 2:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On gen. 23, 2015, 1:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. Matthew Dawson wrote: At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. Vishesh Handa wrote: Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - If the user doesn't want to run them, I'm sure Gentoo can provide some options for that. Compiling them cannot be such a huge cost. - They are already burning CPU time by using a source based distro. This way they might actually catch some bugs and possibly not waste developers time by filing bugs which may have been an issue with their system. I'm not sure if I will approve such patches on packages I maintain. Matthew Dawson wrote: I think we've both stated our piece here, and we aren't going to get further towards a consensus. May I suggest posting this to the general kde-frameworks (or kde-core-devel, I'm not sure what would be better) seeking to make a general policy wrt Frameworks? That way Frameworks has a consistent approach to building tests, whatever way the community decides. In the meantime we should probably merge this patch, as building autotests without finding Qt5Test is only going to break builds. Then packages can be updated with the policy decision by removing the BUILD_TESTING option. The Plasma community should also probably come to a consensus for its packages as well. Your logic is flawed, you say building autotests without finding Qt5Test is only going to break builds. That is correct, just that Qt5Test is being searched for, so your rationale for applying the patch is moot. - Albert --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On gen. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote:
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 1:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. - Andreas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 1:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:43 a.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. Matthew Dawson wrote: At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. Vishesh Handa wrote: Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - If the user doesn't want to run them, I'm sure Gentoo can provide some options for that. Compiling them cannot be such a huge cost. - They are already burning CPU time by using a source based distro. This way they might actually catch some bugs and possibly not waste developers time by filing bugs which may have been an issue with their system. I'm not sure if I will approve such patches on packages I maintain. Matthew Dawson wrote: I think we've both stated our piece here, and we aren't going to get further towards a consensus. May I suggest posting this to the general kde-frameworks (or kde-core-devel, I'm not sure what would be better) seeking to make a general policy wrt Frameworks? That way Frameworks has a consistent approach to building tests, whatever way the community decides. In the meantime we should probably merge this patch, as building autotests without finding Qt5Test is only going to break builds. Then packages can be updated with the policy decision by removing the BUILD_TESTING option. The Plasma community should also probably come to a consensus for its packages as well. Albert Astals Cid wrote: Your logic is flawed, you say building autotests without finding Qt5Test is only going to break builds. That is correct, just that Qt5Test is being searched for, so your rationale for applying the patch is moot. Sorry, I misread the patch. You are correct the build is fine as is. We should discuss this on the mailing list first before applying. - Matthew --- This is an automatically generated e-mail. To reply, visit:
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:43 a.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. Matthew Dawson wrote: At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. Vishesh Handa wrote: Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - If the user doesn't want to run them, I'm sure Gentoo can provide some options for that. Compiling them cannot be such a huge cost. - They are already burning CPU time by using a source based distro. This way they might actually catch some bugs and possibly not waste developers time by filing bugs which may have been an issue with their system. I'm not sure if I will approve such patches on packages I maintain. I think we've both stated our piece here, and we aren't going to get further towards a consensus. May I suggest posting this to the general kde-frameworks (or kde-core-devel, I'm not sure what would be better) seeking to make a general policy wrt Frameworks? That way Frameworks has a consistent approach to building tests, whatever way the community decides. In the meantime we should probably merge this patch, as building autotests without finding Qt5Test is only going to break builds. Then packages can be updated with the policy decision by removing the BUILD_TESTING option. The Plasma community should also probably come to a consensus for its packages as well. - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 2:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 2:48 p.m.) Review request for KDE
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 1:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. - Vishesh --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 8:43 a.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- On Jan. 22, 2015, 2:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 2:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74633 --- Ship it! Not sure why this is suddenly triggering many philosophical discussions about what users should be doing (IMHO, give them choice, this is opensource). Similar changes have been done in most other frameworks long ago, this is most certainly consistent. (see e.g. kcoreaddons - the find_package(Qt5Test) is being done in the autotests subfolder, the same could be done here). - David Faure On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
On Jan. 23, 2015, 2:43 p.m., Vishesh Handa wrote: I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. Matthew Dawson wrote: This is mostly useful on source based distributions (specifically, this patch comes from Gentoo). While in general running tests everywhere would be great, source distro users may not have the cpu time to compile/run tests. Also, some test suites don't work and users may not care to figure out why (for instance, last time I tried enabling tests in Gentoo, binutils failed its suite). For binary distriubtions, I agree they should be running tests (especially since we work to keep them green). But source based distros aren't so clear cut. Andreas Sturmlechner wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. Vishesh Handa wrote: It is even more important for source based distros to be running tests. They generally have very different compile options and flags. What is the point of them running the software and possibly finding bugs, when it could have been caught by just running the tests. Actually, the more I think about this, the more I realize that everyone should be running the autotests. -2 from my side. But I'm not the maintainer of kio. Vishesh Handa wrote: Exactly, packagers do build the tests of course, but that does not mean users of source packages should have a permanent dependency on Qt5Test. On a source based distribution you already have a dependnecy on cmake, the compiler, and many other things. These are only required during build time. Qt5Test is the same. Once the pacakge has been built + tests have been run, Qt5Test can be removed. Matthew Dawson wrote: At least on Gentoo, by default build time dependencies are not automatically removed (though you can remove them if you want). Generally speaking that is the right choice, as you will need cmake/compiler/etc. later to build the package when a version is released. Also, one of the benefits of Gentoo is that the entire development toolchain sticks around, allowing for easy development/bug triaging. Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. Vishesh Handa wrote: Anyways, source based distros won't always run tests, because users won't always want to run them. In a perfect world, I agree that is wrong. In reality, I don't run any test suites across any of my Gentoo installs. So forcing the tests to build just burns CPU time, and is easily patched out by downstreams. I don't think trying to force this will get KDE anywhere. - If the user doesn't want to run them, I'm sure Gentoo can provide some options for that. Compiling them cannot be such a huge cost. - They are already burning CPU time by using a source based distro. This way they might actually catch some bugs and possibly not waste developers time by filing bugs which may have been an issue with their system. I'm not sure if I will approve such patches on packages I maintain. Matthew Dawson wrote: I think we've both stated our piece here, and we aren't going to get further towards a consensus. May I suggest posting this to the general kde-frameworks (or kde-core-devel, I'm not sure what would be better) seeking to make a general policy wrt Frameworks? That way Frameworks has a consistent approach to building tests, whatever way the community decides. In the meantime we should probably merge this patch, as building autotests without finding Qt5Test is only going to break builds. Then packages can be updated with the policy decision by removing the BUILD_TESTING option. The Plasma community should also probably come to a consensus for its packages as well. Albert Astals Cid wrote: Your logic is flawed, you say building autotests without finding Qt5Test is only going to break builds. That is correct, just that Qt5Test is being searched for, so your rationale for applying the patch is moot. Matthew Dawson wrote: Sorry, I misread the patch. You are correct the build is fine as is. We should discuss this on the mailing list first before applying. The Plasma community should also probably come to a consensus for its packages as well. In Plasma most packages
Re: Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/#review74602 --- I'm not against this, but I am curious as to why this is being done. I would think that packagers should be building the tests and running them on their platform and make sure everything passes. We have a strict policy that all tests must always pass. - Vishesh Handa On Jan. 22, 2015, 7:48 p.m., Andreas Sturmlechner wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.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 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- Review request for KDE Frameworks. Repository: kio Description --- [kio] Make tests optional Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing --- This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122206: [kio] Make tests optional
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122206/ --- (Updated Jan. 22, 2015, 7:48 p.m.) Review request for KDE Frameworks. Repository: kio Description (updated) --- [kio] Make tests optional This is a small patch to CMakeLists.txt to only depend on Qt5Test if BUILD_TESTING. Diffs - CMakeLists.txt 7fe0be5d4b2d7d9475a7844b4f8d93fc2f0a00c3 Diff: https://git.reviewboard.kde.org/r/122206/diff/ Testing (updated) --- Thanks, Andreas Sturmlechner ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
