Bugfixes #494
Bugfixes #494
Conversation
tribal-tec
commented
May 26, 2016
tribal-tec
commented
- fix include paths in project header file for generated files within the binary dir
- fix coverage report generation after 5dd04a3 if LCOV_EXCLUDE is empty
| @@ -120,7 +120,7 @@ function(add_coverage_targets TEST_TARGET) | |||
| # 'tests/*' excluded otherwise unit test source file coverage is produced | |||
| add_custom_target(${PROJECT_NAME}-lcov-remove | |||
| COMMAND ${LCOV} -q --remove lcov.info '*.l*' '*.y*' 'tests/*' 'CMake/test/*' | |||
| '*/install/*' 'moc_*' 'qrc_*' ${GENERATED_FILES} '${LCOV_EXCLUDE}' | |||
| '*/install/*' 'moc_*' 'qrc_*' ${GENERATED_FILES} ${LCOV_EXCLUDE} | |||
There was a problem hiding this comment.
No, the quotes are needed at least for Brayns, we added them here: https://github.com/Eyescale/CMake/pull/469/files
There was a problem hiding this comment.
With the quotes and LCOV_EXCLUDE is emtpy, the coverage report is empty as well, hence the modification. I think after the previous PR wrt LCOV_EXCLUDE not set anymore with test files this variable is most of the time empty.
There was a problem hiding this comment.
I got that, but if it is not empty then the quotes are required. Check with Brayns, but the most correct solution is to add a tmp variable which is either empty if LCOV_EXCLUDE is empty or '${LCOV_EXCLUDE}' quoted if it's not.
There was a problem hiding this comment.
There is no target 'BRayns-coverage', so I don't know what you are referring to.
There was a problem hiding this comment.
I am not sure, maybe @favreau remembers the details? What I do know is that this quotes were added for a good reason, and it was observed trying to configure Brayns, but I can't reproduce the problem now. Only ZeroBuf seems to use this variable, but I think we had found more than one occurrences at the time.
|
+1 otherwise if it works |
|
From experimenting I learned that empty quotes ' ' to lcov --remove leads to an empty lcov2.info file. It was never empty so far as we had the tests added, but maybe as BRayns has no tests this was the problem you had. But the coverage report was correct then. No tests, no coverage :) |
