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

Take into account out-of-process executions for code coverage #279

Merged
merged 3 commits into from
May 17, 2019

Conversation

KtorZ
Copy link
Member

@KtorZ KtorZ commented May 17, 2019

Issue Number

#96

Overview

This PR addresses 2 issues:

Comments

NOTE #1
I noticed that we don't get coverage for integration tests which runs in a separate process (like the CLISpec which launches commands and observe the output of the command). So,this is an attempt to instrument the CI to also take the coverage resulting from those out-of-process executions.

NOTE #2
We use some template-haskell in the SQLite implementation to auto-generate a bunch of Haskell types from a few lines of SQL. This is great, but it generates roughly 6000 lines of Haskell, for which, we don't specifically have coverage (and have no way to really know what to cover unless we look at the intermediate splices generated by GHC ... meh). So I've simply "ignored" those lines in CI, so that we don't get half of our source code being flagged as "not covered".

@KtorZ KtorZ self-assigned this May 17, 2019
-- stack as we intend to also get code-coverage from running these commands!
cardanoWallet :: CmdResult r => [String] -> IO r
cardanoWallet args =
command [] "stack" (["exec", "--", "cardano-wallet"] ++ args)
Copy link
Contributor

Choose a reason for hiding this comment

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

@KtorZ KtorZ force-pushed the KtorZ/full-coverage-executables branch 2 times, most recently from fcdedcb to d3d6c57 Compare May 17, 2019 13:07
@KtorZ KtorZ requested a review from paweljakubas May 17, 2019 13:41
Copy link
Contributor

@paweljakubas paweljakubas left a comment

Choose a reason for hiding this comment

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

very neat and smart use of hpc to circumvent wrong code coverage. Nice trick to isolate template haskell bits from regular code, and treat it as covered. As a result, we will not have artificially lowered code coverage

rebasing and stack install hpc ghc in CI and we should be green and able to merge

KtorZ added 2 commits May 17, 2019 16:27
This allows stack to generate a .tix file that can then be combined with other test-suite
.tix file to get the full code-coverage we get as a result of running these integration
tests. Since we fire up shell commands into another process, we don't get coverage for
whatever happens into these shell commands unless we manually ask stack to take the additional
.tix file into account
@KtorZ KtorZ force-pushed the KtorZ/full-coverage-executables branch from 7d7f126 to faf8536 Compare May 17, 2019 14:28
@KtorZ KtorZ force-pushed the KtorZ/full-coverage-executables branch from ed62b95 to 4fdd619 Compare May 17, 2019 15:46
@KtorZ KtorZ merged commit baa1341 into master May 17, 2019
@KtorZ KtorZ deleted the KtorZ/full-coverage-executables branch May 17, 2019 15:47
opt9 pushed a commit to opt9/cardano-sl that referenced this pull request Dec 16, 2019
4052: CBR-499: Remove duplicated SetLastSlots r=mhuesch a=erikd

One last fix for CBR-499.

4055: Increase HTTP client timeout settings r=KtorZ a=Anviking

## Description

Motivated by force_ntp_check timeouts

This is the link to the wallet integration tests : https://github.com/input-output-hk/cardano-sl/blob/c14c018c815732459e8025d5c16dba3bdc186c09/cluster/src/Cardano/Cluster/Environment.hs#L388

This should affect both the wallet and the node monitoring api.

Maybe this shouldn't be merged before 1.5?

<!--- A brief description of this PR and the problem is trying to solve -->

## Linked issue

<!--- Put here the relevant issue from YouTrack -->

cardano-foundation/cardano-wallet#279



Co-authored-by: Michael Hueschen <michael.hueschen@iohk.io>
Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
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.

3 participants