diffpy / libdiffpy

DiffPy C++ library for calculation of PDF and other real-space quantities
Other
7 stars 13 forks source link

activate codecov coverage reports #11

Closed pavoljuhas closed 7 years ago

pavoljuhas commented 7 years ago

The libdiffpy library and its unit tests can be now compiled for coverage tracking by setting "coverage" build mode on scons command line:

$ scons build=coverage lib alltests
# execute test program "alltests".  Because alltests and libdiffpy were built with
# coverage, executing "alltests" creates ".gcda" files with line-hit data.
$ scons build=coverage test

The gcov utility can be then used to create reports per source file.

TODO

@diffpy/contributors - any takers for this job?

CJ-Wright commented 7 years ago

codecov is on conda-forge!

pavoljuhas commented 7 years ago

codecov is on conda-forge

Indeed. However, one of the travis variants (MYUSEMC=false) tests against system packages and does not have Anaconda at all. It is more general to install codecov with pip which will work for all travis runs. PyPI also provides more recent codecov than conda-forge.

CJ-Wright commented 7 years ago

Its very easy to bump to conda-forge version (PRs are always welcome of course).

dragonyanglong commented 7 years ago

Hi Pavol,

James and I have added coverage-mode build to .travis.yml. `script:

Now, to follow your TODO list, we need to use gcov to create reports per source file. Where are these source files for which reports are to be generated?

Another question, for generating report and uploading coverage stats, are these to be done within the .travis.yml? For example: `after_success:

Thank you very much for your help!

pavoljuhas commented 7 years ago

James and I have added coverage-mode build to .travis.yml. - scons -Q build=coverage test

Sounds good - can you also add program build step into the install section?

- scons build=coverage lib alltests

Where are these source files for which reports are to be generated?

Those are the C++ sources (.cpp, .hpp, .ipp) in the src directory.

are these to be done within the .travis.yml?

Yes. You need to make your own fork of this repository and activate the travis build and also the codecov service for your fork. You'll need to also uncomment the final after_success section in .travis.yml, but it probably should not have an extra gcov -a command. The codecov command seems to already execute gcov on its own, see for example https://travis-ci.org/diffpy/diffpy.srreal/jobs/250915975#L1442.

The way it should work is that scons -Q build=coverage test executes the alltests test program. Because it was compiled in coverage mode, the execution of alltests creates .gcno files with line-hit data per each source file. The codecov command then executes gcov to collect the .gcno stats and upload them to codecov.io. This should produce a test-coverage reports for the source files as in https://codecov.io/gh/diffpy/diffpy.structure. A possible problem is that during compilation the sources are duplicated in a temporary build directory and so the paths in gcov data may not match the actual sources. It may be necessary to pass some options for gcov and/or codecov program to get it right.

pavoljuhas commented 7 years ago

@dragonyanglong - how is it going with this issue, are there any major problems or questions? Thank you.

dragonyanglong commented 7 years ago

Hi @pavoljuhas , when we build on travis. It shows error for the first and third build jobs https://travis-ci.org/dragonyanglong/libdiffpy/builds/258605417 Could you please help how we can solve these errors? Thank you!

pavoljuhas commented 7 years ago

@dragonyanglong - please merge diffpy/libdiffpy master 10980c48d3b0412d3084d5f79aa75df317252f40 into your branch. That should fix the build problem.

dragonyanglong commented 7 years ago

@pavoljuhas Thank you very much! Problem solved! I have also added the codecov badges into README. Before merge request, I have two things to confirm:

  1. In README, the codecov badges is linked with my own fork, is that OK? [![codecov](https://codecov.io/gh/dragonyanglong/libdiffpy/branch/master/graph/badge.svg)](https://codecov.io/gh/dragonyanglong/libdiffpy)

  2. Last time you suggest me to change the name into "longyang" when commit something, do you know how to change that? Should I change my Github account name to "longyang"?

Thank you!

pavoljuhas commented 7 years ago

Seems like coverage stats are now up, great! đź‘Ť

In README, the codecov badges is linked with my own fork, is that OK?

Nope - before making pull request please change the URL-s so they correspond to the main repo in diffpy/libdiffpy. We want the badges to be consistent for README in that repository.

Last time you suggest me to change the name into "longyang" when commit something, do you know how to change that?

Yes, you need to set your name and email in the setup of your local git program: https://help.github.com/articles/setting-your-username-in-git/ https://help.github.com/articles/setting-your-commit-email-address-in-git/

$ git config --global user.name "John Doe"
$ git config --global user.email johndoe@example.com

Please make this is consistent for all computers where you use git. No need to change anything on GitHub unless you make commits or merges through their web interface.

gzz0707 commented 7 years ago
                                                                                  Great job, Long!                                                                                                                                                                                                                                                                                                                                        Zizhou Gong                                                                                                                                                                                                                 From: longyangSent: Friday, July 28, 2017 7:55 PMTo: diffpy/libdiffpyReply To: diffpy/libdiffpyCc: James Gong; Team mentionSubject: Re: [diffpy/libdiffpy] activate codecov coverage reports (#11)@pavoljuhas Thank you very much! Problem solved! I have also added the codecov badges into README. Before merge request, I have two things to confirm:

In README, the codecov badges is linked with my own fork, is that OK? codecov

Last time you suggest me to change the name into "longyang" when commit something, do you know how to change that? Should I change my Github account name to "longyang"?

Thank you!

—You are receiving this because you are on a team that was mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/diffpy/libdiffpy","title":"diffpy/libdiffpy","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/diffpy/libdiffpy"}},"updates":{"snippets":[{"icon":"PERSON","message":"@dragonyanglong in #11: @pavoljuhas Thank you very much! Problem solved! I have also added the codecov badges into README. Before merge request, I have two things to confirm:\r\n1. In README, the codecov badges is linked with my own fork, is that OK?\r\n[![codecov](https://codecov.io/gh/dragonyanglong/libdiffpy/branch/master/graph/badge.svg)](https://codecov.io/gh/dragonyanglong/libdiffpy)\r\n\r\n2. Last time you suggest me to change the name into \"longyang\" when commit something, do you know how to change that? Should I change my Github account name to \"longyang\"?\r\n\r\nThank you!"}],"action":{"name":"View Issue","url":"https://github.com/diffpy/libdiffpy/issues/11#issuecomment-318786794"}}}

dragonyanglong commented 7 years ago

Hi @pavoljuhas , thanks for the help.

  1. I have already changed the URL-s into diffpy/libdiffpy in README. [![codecov](https://codecov.io/gh/diffpy/libdiffpy/branch/master/graph/badge.svg)](https://codecov.io/gh/diffpy/libdiffpy) Now at my own fork, in README, the codecov badge status shows "unkown" because I don't have permission to diffpy/libdiffpy in codecov. So I guess after pull request and merge into the main diffpy/libdiffpy, it will show correctly on diffpy/libdiffpy/README. Ready for pull request?

  2. For the name for commit, I tried many ways to change the user.name for git commit, and my name and email are set correctly in git config (global and each repository). However, on the web page, it still always shows my github username "dragonyanglong" for my commits. I also tried adding author info when "git commit": git commit -m "test" --author="Long Yang <dragonyang92@gmail.com>". And the author info shows correctly for this commit [master dae04f4] test Author: Long Yang <dragonyang92@gmail.com> but still doesn't work.

pavoljuhas commented 7 years ago

Now at my own fork, in README, the codecov badge status shows "unkown" because I don't have permission to diffpy/libdiffpy in codecov. So I guess after pull request and merge into the main diffpy/libdiffpy, it will show correctly on diffpy/libdiffpy/README. Ready for pull request?

Almost there. I found a couple of small corrections which I will post about shortly within the relevant commits. Also, please create a new branch with a descriptive name, for example, "activate-coverage-reports" and rebase it off the master branch in the main repository. Provided that your current branch points to d95c14977bcb33935ebc98891dbf5b39d2b39b99 this can be done as follows:

$ git checkout -b activate-coverage-reports
# do the corrections for e44bc3d3cf ...
# and then commit them with adjusted commit message
$ git commit --squash=e44bc3d3cf

# now rebase off the main repository master which is 10980c48
$ git rebase -i --autosquash 10980c48
# now squash the "edit install" commit with its correction
# and also squash the 2 commits that modify README file
# After rebasing you should have 2 corrected commits that
# are on top of the diffpy/libdiffpy master.

# Check if the 2 new commits look good as for their content and commit messages.
# You can repeat the rebase another time if anything needs correction.
# Finally, push the "active-coverage-reports" branch to your GH fork
# and open the PR. 

#  (assuming the remote for your fork is "long")
$ git push long -u activate-coverage-reports

For the name for commit, I tried many ways to change the user.name for git commit, and my name and email are set correctly in git config ...

Actually your name and email in the current commits look good. What matters is the output from git log rather than what shows on the GH page:

$ git log long/master                
commit d95c14977bcb33935ebc98891dbf5b39d2b39b99 (HEAD -> mlong, long/master)
Author: Long Yang <ly2364@columbia.edu>
...

I was suggesting to avoid incomplete name, for example

$ git -C where/is/diffpy.structure log --all --author=lyang
commit 3f29bda54815f060c1569dfadeb788b5b8211546
Author: lyang <ly2364@columbia.edu>
...
pavoljuhas commented 7 years ago

see https://github.com/dragonyanglong/libdiffpy/commit/e44bc3d3cf18d86e869524ca475247b884e1cb79#commitcomment-23399432

dragonyanglong commented 7 years ago

Thank you very much for these detailed explanations, Pavol! PR requested, if any problem, please let me know. Really appreciate your time and mentoring!

pavoljuhas commented 7 years ago

PR looks good, merged. Thank you for your help!