Merge lp:~dobey/indicator-display/fix-coverage into lp:indicator-display

Proposed by dobey on 2017-02-07
Status: Merged
Approved by: dobey on 2017-02-10
Approved revision: 27
Merged at revision: 28
Proposed branch: lp:~dobey/indicator-display/fix-coverage
Merge into: lp:indicator-display
Prerequisite: lp:~dobey/indicator-display/fix-i18n
Diff against target: 125 lines (+34/-18)
5 files modified
CMakeLists.txt (+15/-18)
debian/control (+1/-0)
tests/CMakeLists.txt (+5/-0)
tests/integration/CMakeLists.txt (+6/-0)
tests/unit/CMakeLists.txt (+7/-0)
To merge this branch: bzr merge lp:~dobey/indicator-display/fix-coverage
Reviewer Review Type Date Requested Status
unity-api-1-bot continuous-integration Approve on 2017-02-13
Pete Woods (community) Approve on 2017-02-10
Ted Gould (community) 2017-02-07 Needs Information on 2017-02-07
Review via email: mp+316601@code.launchpad.net

Commit message

Make coverage reporting work.

To post a comment you must log in.
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:26
https://jenkins.canonical.com/unity-api-1/job/lp-indicator-display-ci/26/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1617
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1624
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1402/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1402/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1402/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1402/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1402/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1402
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1402/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-indicator-display-ci/26/rebuild

review: Approve (continuous-integration)
Ted Gould (ted) wrote :

I think that instead of defining the variable in the parent scope, it should instead be a global property. This way you don't have to track the level of recursion in each file. Also, with set property you can do an append:

set_property(GLOBAL APPEND PROPERTY COVERAGE_TEST_TARGETS ${TEST_NAME})

review: Needs Information
Charles Kerr (charlesk) wrote :

Keeper uses the cascading-parent-scope approach as an alternative to hardcoding all the test names in the toplevel CMakeLists.txt file, and I suggested it here for the same reason.

If it works, the global property approach sounds cleaner than the previous two approaches.

Pete Woods (pete-woods) :
review: Needs Fixing
27. By dobey on 2017-02-10

Remove hard-coded -g option.

Pete Woods (pete-woods) :
review: Approve
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:27
https://jenkins.canonical.com/unity-api-1/job/lp-indicator-display-ci/28/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1668
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1675
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1450
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1450/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1450
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/1450/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1450
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1450/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1450
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/1450/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1450
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/1450/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1450
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/1450/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-indicator-display-ci/28/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2016-12-07 17:06:29 +0000
3+++ CMakeLists.txt 2017-02-10 16:17:38 +0000
4@@ -13,9 +13,6 @@
5 set(SERVICE_LIB ${PACKAGE})
6 set(SERVICE_EXEC "${PACKAGE}-service")
7
8-option(enable_tests "Build the package's automatic tests." ON)
9-option(enable_coverage "Generate code coverage reports." ON)
10-
11 ##
12 ## GNU standard paths
13 ##
14@@ -67,22 +64,22 @@
15 list(APPEND CXX_WARNING_ARGS -Wall -Wextra -Wpedantic)
16 endif()
17
18-add_compile_options(-std=c++14 -fPIC -g)
19-
20-##
21-## Testing & Coverage
22-##
23-
24-if(${enable_tests})
25- enable_testing()
26- if(${enable_coverage})
27- find_package(CoverageReport)
28- endif()
29-endif()
30+add_compile_options(-std=c++14 -fPIC)
31
32 add_subdirectory(src)
33 add_subdirectory(data)
34 add_subdirectory(po)
35-if (${enable_tests})
36- add_subdirectory(tests)
37-endif ()
38+
39+##
40+## Testing & Coverage
41+##
42+
43+enable_testing()
44+add_subdirectory(tests)
45+find_package(CoverageReport)
46+ENABLE_COVERAGE_REPORT(
47+ TARGETS ${SERVICE_LIB} ${SERVICE_EXEC}
48+ TESTS ${COVERAGE_TEST_TARGETS}
49+ FILTER /usr/include ${CMAKE_BINARY_DIR}/*
50+)
51+
52
53=== modified file 'debian/control'
54--- debian/control 2016-12-07 17:06:29 +0000
55+++ debian/control 2017-02-10 16:17:38 +0000
56@@ -10,6 +10,7 @@
57 libgudev-1.0-dev,
58 libproperties-cpp-dev,
59 # for coverage reports
60+ gcovr,
61 lcov,
62 # for tests
63 qt5-default,
64
65=== modified file 'tests/CMakeLists.txt'
66--- tests/CMakeLists.txt 2016-03-11 20:06:34 +0000
67+++ tests/CMakeLists.txt 2017-02-10 16:17:38 +0000
68@@ -33,3 +33,8 @@
69 add_subdirectory(integration)
70 add_subdirectory(unit)
71 add_subdirectory(utils)
72+
73+set(COVERAGE_TEST_TARGETS
74+ ${COVERAGE_TEST_TARGETS}
75+ PARENT_SCOPE
76+)
77
78=== modified file 'tests/integration/CMakeLists.txt'
79--- tests/integration/CMakeLists.txt 2016-03-16 00:25:33 +0000
80+++ tests/integration/CMakeLists.txt 2017-02-10 16:17:38 +0000
81@@ -16,9 +16,15 @@
82
83 function(add_qt_test_by_name name)
84 set(TEST_NAME ${name})
85+ set(COVERAGE_TEST_TARGETS ${COVERAGE_TEST_TARGETS} ${TEST_NAME} PARENT_SCOPE)
86 add_executable (${TEST_NAME} ${TEST_NAME}.cpp)
87 add_test(${TEST_NAME} ${TEST_NAME})
88 set_property(TEST ${TEST_NAME} APPEND PROPERTY ENVIRONMENT ${CTEST_ENVIRONMENT})
89 target_link_libraries(${TEST_NAME} ${SERVICE_LINK_LIBRARIES} ${QT_LINK_LIBRARIES} ${TEST_LINK_LIBRARIES} ${THREAD_LINK_LIBRARIES})
90 endfunction()
91 add_qt_test_by_name(usb-manager-test)
92+
93+set(COVERAGE_TEST_TARGETS
94+ ${COVERAGE_TEST_TARGETS}
95+ PARENT_SCOPE
96+)
97
98=== modified file 'tests/unit/CMakeLists.txt'
99--- tests/unit/CMakeLists.txt 2016-04-21 00:57:03 +0000
100+++ tests/unit/CMakeLists.txt 2017-02-10 16:17:38 +0000
101@@ -20,6 +20,7 @@
102
103 function(add_test_by_name name)
104 set(TEST_NAME ${name})
105+ set(COVERAGE_TEST_TARGETS ${COVERAGE_TEST_TARGETS} ${TEST_NAME} PARENT_SCOPE)
106 add_executable (${TEST_NAME} ${TEST_NAME}.cpp)
107 add_test(${TEST_NAME} ${TEST_NAME})
108 set_property(TEST ${TEST_NAME} APPEND PROPERTY ENVIRONMENT ${CTEST_ENVIRONMENT})
109@@ -30,6 +31,7 @@
110
111 function(add_qt_test_by_name name)
112 set(TEST_NAME ${name})
113+ set(COVERAGE_TEST_TARGETS ${COVERAGE_TEST_TARGETS} ${TEST_NAME} PARENT_SCOPE)
114 add_executable (${TEST_NAME} ${TEST_NAME}.cpp)
115 add_test(${TEST_NAME} ${TEST_NAME})
116 set_property(TEST ${TEST_NAME} APPEND PROPERTY ENVIRONMENT ${CTEST_ENVIRONMENT})
117@@ -37,3 +39,8 @@
118 endfunction()
119 add_qt_test_by_name(greeter-test)
120 add_qt_test_by_name(usb-snap-test)
121+
122+set(COVERAGE_TEST_TARGETS
123+ ${COVERAGE_TEST_TARGETS}
124+ PARENT_SCOPE
125+)

Subscribers

People subscribed via source and target branches