-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Automated "test_all" target and Code Coverage #690
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this. It all looks good to me. I'll give it a quick run on macOS, too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible/unreasonable to enable coverage analysis in the CI pipeline builds?
I just tested successfully on macOS with compilers from Homebrew. In addition to setting |
Once the gcov/llvm-cov selection logic is fixed, I'm enthusiastically +1 on this |
Something to potentially enhance down the line would be to exclude irrelevant bits from the metrics. The worst offender is the Google Test source code in If it's easy to do that, it would be great to fold that in here |
It may also make sense to not measure coverage of the test files as such, and only report on |
I discovered in testing this on my Mac that the exclusions that are supposed to limit results to just I'm trying to patch that by resolving the paths, but it's not working as desired. |
There's maybe some race condition if one runs |
My remaining comments are probably fine to ignore. This achieves its purpose, and isn't otherwise making anything else around it worse. Let's just get it in, reap the benefits, and we can fix nitty-gritty issues as they become practical concerns. |
- add changelog comments to coverage model - modify gcovr functions to ignore test failures - fix logic in selecting llvm-cov or gcov - change coverage compiler flags from -O0 to -Og
e34a944
to
d2dfaac
Compare
@PhilMiller I've modified the CMake module to include your suggestions, but particularly, removed all the language-specific flags and used |
This PR reworks the
test_all
target to automatically amalgamate all tests defined throughngen_add_test
, and builds on #348 to add code coverage usinggcovr
instead oflcov
. In addition to usinggcovr
, modifications were made toCodeCoverage.cmake
to support usingllvm-cov
withgcov
emulation.Additions
NGEN_WITH_COVERAGE
. Dependent onNGEN_WITH_TESTS
, and allows building thengen_coverage_*
targets.cmake/CodeCoverage.cmake
module, which provides functions for generating HTML and XML code coverage documents.ngen_coverage_html
andngen_coverage_xml
for HTML and XML documents respectively.The output files for these targets are outputted to:
<build_directory>/<target_name>/index.html
<build_directory>/<target_name>.xml
.Changes
test_all
is now automatically generated throughngen_add_test
rather than explicitly at the end of thetest/CMakeLists.txt
file.Notes
When generating code coverage documents, the
.gcda
files are generated alongside the source object files inCMakeFiles/
directories. As a result, an out-of-source build is necessary, since it is not worth attempting to clean up these files in an in-source build. Moreover, to generate successive coverage documents, the gcov data files need to be deleted beforehand, so deleting the out-of-source build directory is the recommended approach.If using a custom compiler with (e.g.)
-DCMAKE_CXX_COMPILER=/path/to/bin/gcc-23
, the path to the corresponding coverage analysis tool needs to be set with-DGCOV_PATH=/path/to/bin/gcov-23
, or-DLLVM_COV_PATH=/directory/with/llvm/bin/llvm-cov-42
as appropriateExample Coverage:
Testing
Checklist
Target Environment support