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

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

do we really need KDEInstallDirs and KDECMakeSettings?

himanshuvishwakarma added inline comments.Mar 16 2018, 7:54 PM
CMakeLists.txt
35

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

39

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...