Differential D11391

ecm_add_test method is added
ClosedPublic
Actions

Authored by himanshuvishwakarma on Mar 16 2018, 2:55 PM.

Details

Reviewers
jjazeix
dmadaan
timotheegiet
echarruau
Group Reviewers
GCompris
Summary

In this diff,
I removed the enable testing & add-testing and others method for the adding the unit test. Now, we have to add the test by using the one method i.e. ecm_add_test in cmake.

by this change, extra-cmake-modules is required for the building test. So I updated the HACKING manual also.

Diff Detail

Repository
R2 GCompris
Lint
Lint Skipped
Unit
Unit Tests Skipped
Restricted Application added a project: KDE Edu. · View Herald TranscriptMar 16 2018, 2:55 PM
Restricted Application added a subscriber: KDE Edu. · View Herald Transcript
himanshuvishwakarma requested review of this revision.Mar 16 2018, 2:55 PM
jjazeix added inline comments.Mar 16 2018, 7:07 PM
CMakeLists.txt
35 ↗(On Diff #29679)

I'd rather not force to install ECM if not needed for building the main executable.
On other OS than linux, it may be difficult to find and install.

39 ↗(On Diff #29679)

do we really need KDEInstallDirs and KDECMakeSettings?

himanshuvishwakarma added inline comments.Mar 16 2018, 7:54 PM
CMakeLists.txt
35 ↗(On Diff #29679)

Okay, i will make it optional, If we make ECM as optional then our test will also be optional.

39 ↗(On Diff #29679)

Yaa, I think so because of the without KDEInstallDirs and KDECMakeSettings, ecm_add_test method of the module ECMAddTests is not working. : )

jjazeix accepted this revision.Mar 17 2018, 8:30 PM

I pushed an equivalent of it in https://commits.kde.org/gcompris/4043239e98e32177085449e2757b8dc6462cb621
Now unit tests are optional so you need to pass -DBUILD_TESTING to build the library and unit tests.
I didn't added KDEInstallDirs and KDECMakeSettings as it was not needed

This revision is now accepted and ready to land.Mar 17 2018, 8:30 PM
jjazeix closed this revision.Mar 17 2018, 8:30 PM
himanshuvishwakarma updated this revision to Diff 29784.Mar 18 2018, 1:27 AM
himanshuvishwakarma edited the summary of this revision. (Show Details)

Now, extra-cmake-modules are not compulsory to compile the application but it required to run the unit test and other things.

himanshuvishwakarma added a comment.Mar 18 2018, 1:32 AM

I am sorry, I haven't seen that you push it and make necessary changes as required.

Revision Contents
Changeset List

PathPackages
MCMakeLists.txt (7 lines)
MHACKING (23 lines)
Mtests/core/CMakeLists.txt (4 lines)

Diff 29784

View Options

CMakeLists.txt

Loading...
View Options

HACKING

Loading...
View Options

tests/core/CMakeLists.txt

Loading...