Skip to content
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

Add coverage testing. #903

Merged
merged 1 commit into from
Apr 18, 2015
Merged

Add coverage testing. #903

merged 1 commit into from
Apr 18, 2015

Conversation

dmakarov
Copy link
Contributor

This PR enables collecting gcov code coverage statistics and pushing them to coveralls.io. It also adds the coveralls badge to README. Someone with access to ldc-developers account has to create/connect a coveralls.io account to ldc-develoepr/ldc repository to make this work. This PR should close the issue #380. I enabled test coverage for llvm 3.5 build, but of course any other one can be used as well. Please, review.

#
set(TEST_COVERAGE OFF CACHE BOOL "instrument compiler for code coverage analysis")
if(TEST_COVERAGE)
append("-O0 -g -fprofile-arcs -ftest-coverage" EXTRA_CXXFLAGS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a guard before appending options:

if(CMAKE_COMPILER_IS_GNUCXX OR (${CMAKE_CXX_COMPILER_ID} STREQUAL "Clang"))

the else branch should be a warning that coverage testing is not available.

@redstar
Copy link
Member

redstar commented Apr 18, 2015

Otherwise looks good to me. Thanks!!!!

@dmakarov
Copy link
Contributor Author

I added the guard, but don't merge yet, please. I want to mute the verbose gcov output by redirecting and also to enable after_success for test coverage builds only. I'll post another message here, when it's done.

@dmakarov
Copy link
Contributor Author

Now everything seems to be ok. Pretty good coverage 85.92%.

@dnadlinger
Copy link
Member

Let's give this a go. I enabled coveralls.io for ldc-developers/ldc.

dnadlinger added a commit that referenced this pull request Apr 18, 2015
@dnadlinger dnadlinger merged commit b9d13e8 into ldc-developers:master Apr 18, 2015
@dnadlinger
Copy link
Member

By the way, to close an issue, just include "GitHub: Closes #380" in the commit message (the "GitHub" part actually isn't needed, but I like to include it for clarity should we ever move issue trackers).

@dmakarov
Copy link
Contributor Author

OK, I'll do it like that next time I'll commit something that closes an issue. Unless you already have it, might be a good idea to keep a checklist for contributors with all these details. I know there is a wiki page about how to contribute, but somehow there are several steps just to reach from the repository's readme.

@dmakarov dmakarov deleted the cover branch April 18, 2015 16:28
@dnadlinger
Copy link
Member

Seems like the results appeared on Coveralls.io just fine. Thanks for the great work! (For the badge, we'll need to wait until camo.githubusercontent.com updates its cache.)

@JohanEngelen
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants