Adding kcov code coverage to chain and core modules #115

percytheprefect · 2017-09-04T17:47:58Z

This pull request implements code coverage for the core and chain modules of grin. It follows this guide to implement the test coverage.

If anyone knows of a better way to make code coverage easily accessible for grin let me know.

codecov-io · 2017-09-04T18:24:08Z

Codecov Report

❗ No coverage uploaded for pull request base (master@da4b349). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             master   #115   +/-   ##
=======================================
  Coverage          ?     0%           
=======================================
  Files             ?     39           
  Lines             ?   4580           
  Branches          ?      0           
=======================================
  Hits              ?      0           
  Misses            ?   4580           
  Partials          ?      0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da4b349...f3d3b34. Read the comment docs.

.travis.yml

+after_success: |
+  pwd &&
+  echo "HERE" &&
+  wget https://github.com/percytheprefect/kcov/archive/master.tar.gz &&


sesam · 2017-11-25T10:23:13Z

@percytheprefect Nice initiative.

Guide is dated July 25th, 2016.
- ~~Could you try it out with the official repo instead of a fork?~~ EDIT: Sorry, I finally understood we're on the most recent version.
- Maybe you (or me) should ask the author for ideas on what might have changed since?
~~Is this something I'd use locally only?~~ EDIT: no, it's run on Travis CI servers.
Add a badge ("shield") in README.md? Recent article https://medium.com/@Razican/continuous-integration-and-code-coverage-report-for-a-rust-project-5dfd4d68fbe5 says basically the same for kcov.
And a more recent official commentary on how to track code coverage Add the ability for crate authors to add code coverage badges to their crate rust-lang/crates.io#706 (dated May 2017).

I'm interested to work more on this. I'm sesam on grin's gitter chat.

sesam · 2017-11-27T14:10:23Z

I found https://github.com/huonw/travis-cargo which probably does more or less the same, but may be even more outdated. The opensource way to solve this would be upstreaming fixes to @huonw or someone who can take over that burden.

I also found https://crates.io/crates/cargo-tarpaulin (linux x86-only) and tried:

apt-get update && apt-get install libssl-dev
cargo install cargo-tarpaulin
cargo tarpaulin -v

with no success when tested on mostly vanilla Debian Jessie.

sesam · 2017-12-12T10:21:26Z

@codecov-io aboev has outdated links. The link to the "full report" is https://codecov.io/gh/mimblewimble/grin/pull/115 and that is still reporting "missing base report".
Maybe loggin in as the profile https://codecov.io/gh/mimblewimble and doing (something) can fix this?

sdtsui · 2017-12-24T06:34:40Z

@sesam :
Thank you for the links and research -- saved me some time getting oriented. :)

I think tarpaulin is a good option: actively maintained and aiming for functionality that kcov won't support. Unfortunately, it's not complete. An 0.5.6 release was pushed last week and the build is still failing. Here's the original Reddit thread. What do you think of looking at it after a 1.0, perhaps only if we need more coverage options (like condition coverage)?

Regarding huonw/travis-cargo, the main negative I see is it's not actively maintained. Here's one of its most recent PRs, from a few months ago:

Most recent commits are >2yrs old.

sdtsui

@percytheprefect @ignopeverell :

Any plans for this PR? I'm interested in picking this up or collaborating by chatting regularly with @sesam.
Regarding this PR's approach to coverage: any interest in maintaining our own fork of kcov? Re: @percytheprefect 's comment:

If you want to fork the repo so that we can use your fork as our reference repository that is fine too.

I asked around and several projects appear to have accepted SimonKagstrom/kcov as a standard, if somewhat slow. From another blog, jbp.io, using LLVM:

Prior to this work, I used kcov. This produced good output, but each test process took around three seconds to start. One of the test suites runs around 1000 processes, so this meant a full test run took almost an hour (compared to 50 seconds without coverage).

So if we want to stick to the approach put forth in this PR, I think using kcov should be fine.

Unrelated to this PR, in Unit Tests for API endpoints #434 @sesam mentioned adding a fuzzer using quickcheck -- I'd be interested in exploring this as well. Do we have the same concerns with ownership of forks? It feels like a separate project to me -- might it make sense as a separate issue?

I found kennytm/cov, which is more recent and appears to have some acceptance in the community. See this rfc to see the progression over the last few years. It also supports more targets than linux/x86.

You can try it locally, like this:

cargo install cargo-cov
cargo cov clean
cargo cov test
cargo cov report --open

I figure it's best to check with you two, as I'm new to the project. Do the results here, for core/src look accurate (or at least not wildly off...)?

If so, I can explore automating the processing/upload of coverage results, similar to what is described in the jbp.io blog post linked above.

For now, we can use this locally to help make decisions about what needs more testing. Thoughts?

ignopeverell · 2017-12-24T19:21:34Z

Hey @sdtsui, thanks for picking this up with @sesam. I would have no problem having our own fork of kcov but it sounds like cargo-cov is a better option now. Am I reading this right? The report you pasted looks pretty good to me and seems about right. Is there any publicly hosted option for cargo-cov? Could we somehow integrate it with travis?

sesam · 2017-12-24T22:33:29Z

I might try to convince @hendi once back from newyears busyness to set up a cargo-cov on his machines

…

On Dec 24, 2017 20:21, "Ignotus Peverell" ***@***.***> wrote: Hey @sdtsui <https://github.com/sdtsui>, thanks for picking this up with @sesam <https://github.com/sesam>. I would have no problem having our own fork of kcov but it sounds like cargo-cov is a better option now. Am I reading this right? The report you pasted looks pretty good to me and seems about right. Is there any publicly hosted option for cargo-cov? Could we somehow integrate it with travis? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#115 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAi2aXVkTI1BLU0xsmHh_zBQG49_c-mks5tDqQ-gaJpZM4PMJpi> .

sdtsui · 2017-12-25T08:11:51Z

@ignopeverell I did some more reading today, looking deeper into the different projects we could use.

cargo-cov maintainer's progress on coveralls support:
Coveralls support kennytm/cov#3

Anyway this project (kennytm/cov) is currently very inactive because the coverage is useless when unused generic functions are simply ignored...

also parity's solidity testing framework, sol-rs on this topic:
[WIP] Coverage Analysis openethereum/sol-rs#20

Apparently there is an issue with Rust profiling picking up on generic functions (see this issue in cargo-cov). So this approach may work for some basic coverage statistics, but not a long-term / ideal solution by any stretch.

Hosted publically on crates.io:
https://crates.io/crates/cargo-cov

I'm not sure if we should use this after all, especially given kennytm's comment. Let me know how you feel about losing coverage for unused generic funcs: could be a deal breaker.

Also note that kennytm also has another project: cargo-kcov that has <20k downloads (kcov, according to its github badge has 900k+ now), and is Linux only (no future macOS or other support).

@ignopeverell
So perhaps what we have here (+forking kcov) is best? Individual devs can still use cargo-cov as-is. If so, I can take this commit and add changes (including the badge) in anticipation of a mimblewimble/kcov repo, wrap this up tomorrow.

Sound ok?
/cc @sesam

Fixing bug in .yml file Adding cargo test --no-run command to after_success Removing comment, adding pwd to see what working directory is Debugging working directory for .yml file Replacing binutils with libbfd-dev Removing libbfd-dev because it fails the build Adding test coverage to rest of grin modules

percytheprefect · 2017-12-26T04:50:38Z

I think we should choose the simplest solution. We do not want to allocate many resources to fiddle with code coverage. @sdtsui you will need to rebase your #545 onto this new commit. I have added coverage to other modules in the grin codebase.

mentioned adding a fuzzer using quickcheck -- I'd be interested in exploring this as well. Do we have the same concerns with ownership of forks? It feels like a separate project to me -- might it make sense as a separate issue?

This is a good idea. Grin's testing procedures have been lacking. Property based testing, in conjunction with a few hard coded test vectors, is a proven way to build high quality software. I'm guessing quickcheck will unearth some tricky bugs in our implementation. We should start with writing properties for our cryptographic primitives and serializers and work our way out from there.

https://github.com/BurntSushi/quickcheck

ignopeverell · 2017-12-27T02:11:20Z

Ended up merging #545, please PR on top of master to add new modules, improvements, etc.

Definitely agree we should ramp up testing as much as possible and quickcheck has its place there.

.travis.yml

+  cargo test --no-run &&
+  cd ../
+  pwd &&
+  kcov --exclude-pattern=/.cargo,/usr/lib --verify target/cov target/debug/grin_core* && 


ignopeverell reviewed Sep 5, 2017

View reviewed changes

.travis.yml Outdated

after_success: |

pwd &&

echo "HERE" &&

wget https://github.com/percytheprefect/kcov/archive/master.tar.gz &&

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

sesam mentioned this pull request Dec 12, 2017

Unit Tests for API endpoints #434

Closed

sdtsui reviewed Dec 24, 2017

View reviewed changes

sdtsui mentioned this pull request Dec 25, 2017

Add kcov #545

Merged

1 task

percytheprefect force-pushed the test_coverage branch from 071a595 to f3d3b34 Compare December 26, 2017 04:38

ignopeverell closed this Dec 27, 2017

sdtsui reviewed Dec 27, 2017

View reviewed changes

.travis.yml

cargo test --no-run &&

cd ../

pwd &&

kcov --exclude-pattern=/.cargo,/usr/lib --verify target/cov target/debug/grin_core* &&

This comment was marked as spam.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adding kcov code coverage to chain and core modules #115

Adding kcov code coverage to chain and core modules #115

Uh oh!

percytheprefect commented Sep 4, 2017

Uh oh!

codecov-io commented Sep 4, 2017 •

edited

Loading

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

sesam commented Nov 25, 2017 •

edited

Loading

Uh oh!

sesam commented Nov 27, 2017 •

edited

Loading

Uh oh!

sesam commented Dec 12, 2017

Uh oh!

sdtsui commented Dec 24, 2017 •

edited

Loading

Uh oh!

sdtsui left a comment •

edited

Loading

Uh oh!

ignopeverell commented Dec 24, 2017

Uh oh!

sesam commented Dec 24, 2017 via email

Uh oh!

sdtsui commented Dec 25, 2017

Uh oh!

percytheprefect commented Dec 26, 2017

Uh oh!

ignopeverell commented Dec 27, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

Uh oh!

Adding kcov code coverage to chain and core modules #115

Adding kcov code coverage to chain and core modules #115

Uh oh!

Conversation

percytheprefect commented Sep 4, 2017

Uh oh!

codecov-io commented Sep 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

sesam commented Nov 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sesam commented Nov 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sesam commented Dec 12, 2017

Uh oh!

sdtsui commented Dec 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdtsui left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ignopeverell commented Dec 24, 2017

Uh oh!

sesam commented Dec 24, 2017 via email

Uh oh!

sdtsui commented Dec 25, 2017

Uh oh!

percytheprefect commented Dec 26, 2017

Uh oh!

ignopeverell commented Dec 27, 2017

Uh oh!

This comment was marked as spam.

Uh oh!

Uh oh!

codecov-io commented Sep 4, 2017 •

edited

Loading

sesam commented Nov 25, 2017 •

edited

Loading

sesam commented Nov 27, 2017 •

edited

Loading

sdtsui commented Dec 24, 2017 •

edited

Loading

sdtsui left a comment •

edited

Loading