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

Code coverage #148

Open
4 tasks
mbr opened this issue Jul 18, 2018 · 3 comments
Open
4 tasks

Code coverage #148

mbr opened this issue Jul 18, 2018 · 3 comments
Labels

Comments

@mbr
Copy link
Contributor

mbr commented Jul 18, 2018

Inspired by @DrPeterVanNostrand's potential bug due to a never-taken branch, I gave coverage a try. Other crates are using some form of coverage (e.g. https://github.com/ctz/rustls does not only feature an awesome logo, but over a hundred lines of custom tooling to enable this functionality), but it is far from trivial to get it working.

Here's what I tried this morning, by trying to get coverage for a toy crate that uses some crates and features we want in hbbft in the future:

There are more things to try (e.g. doing a clean step-by-step of https://jbp.io/2017/07/19/measuring-test-coverage-of-rust-programs), but as a whole, I am not very happy about the current state of tooling in this regard.

I am doing something wrong though because shortly before me giving up, this finally worked:

  1. Install cargo-make (cargo install cargo-make),
  2. run cargo test --no-run to build the tests,
  3. create coverage info using cargo make coverage-flow.

Now it is possible to open target/coverage/index.html a browser.

Probable next steps:

  • Getting cargo-make to work with tests compiled with --release (will take too long otherwise).
  • Checking if the whole thing even runs on hbbft.
  • Double-checking whether the output is correct.
  • Finding a way to conveniently display the results, either by building HTML files (gh_pages or using some service like https://coveralls.io/).

Comments welcome!

@vkomenda
Copy link
Contributor

vkomenda commented Aug 10, 2018

cargo make coverage-flow only computes coverage of the internal hbbft tests for me:

running 3 tests
test broadcast::merkle::tests::test_merkle ... ok
test dynamic_honey_badger::votes::tests::test_committed_votes ... ok
test dynamic_honey_badger::votes::tests::test_pending_votes ... ok

This is due to the setting of the test coverage filter being set as follows:

CARGO_MAKE_TEST_COVERAGE_BINARY_FILTER_REGEX=hbbft-[a-z0-9]*$\|test_[a-z0-9]*-[a-z0-9]*$\|[a-z0-9]*_test-[a-z0-9]*$

This variable is set in the initial environment of cargo-make. This could be made to work for the integration tests by appending _test to every test file name.

I think that computing coverage on all executable files in target/release would be even better. There shouldn't be any naming constraints on test file names.

@vkomenda
Copy link
Contributor

For some reason cargo make coverage, when applied to all executable files in target/release, returns 0% coverage for all integration tests and 91.2% for the unit tests in the hbbft binary. However all those executables are compiled with debug info and are not stripped. It would be interesting to know why it still works on the main binary.

It does work for the integration tests in target/debug. The execution time is completely unrealistic though.

@vkomenda
Copy link
Contributor

It took two and a half days on my laptop to run all tests in the current master in debug mode to compute coverage with cargo make coverage: 84.5%. Not a bad figure for a start. Let's see how the proptest-based test framework improves on that mark.

cargo make coverage always runs an installation script to set up kcov. This looks unnecessary in our Travis. We could install kcov when setting up the image and then just use kcov without the cargo-make tool.

@afck afck added the test label Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants