Skip to content

Add coverage events #1167

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

Merged
merged 2 commits into from
Dec 20, 2018
Merged

Add coverage events #1167

merged 2 commits into from
Dec 20, 2018

Conversation

alaibe
Copy link
Contributor

@alaibe alaibe commented Dec 7, 2018

New strategy for code coverage:

It requires solidity with emit keyword for event (0.4.21 or higher)
Instead of using the trace from the vm, use events that we inject into the contract.

Change required if you deploy your contract manually, instead of doing:
MyContract.deploy(deployArgs).send(sendArgs) do: embark.deploy(MyContract, deployArgs, sendArgs)

Otherwise we cannot fetch the events associated to this contract

Pros:

  • 100% accurate on line count
  • Support library
  • Support all cases

Cons:

Possible improvements:

  • Do not emit event for function and branch, in theory the statement are enough, so gas consumption would be reduced

@alaibe alaibe force-pushed the feat/coverage-events branch from 1590620 to cfbeb13 Compare December 7, 2018 14:18
Copy link
Collaborator

@jrainville jrainville left a comment

Choose a reason for hiding this comment

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

I'd say we should get rid of the function and branch events, as they are pretty much duplicated because of the statements.
Also, why did you remove the coverage app?

@alaibe
Copy link
Contributor Author

alaibe commented Dec 7, 2018

I'd say we should get rid of the function and branch events, as they are pretty much duplicated because of the statements.

See PR description, I will do it in a following PR, this is already quite complex and big.

@alaibe alaibe force-pushed the feat/coverage-events branch from 58209a8 to b2e79ce Compare December 7, 2018 14:55
@iurimatias
Copy link
Collaborator

@alaibe can you rebase please.

@alaibe alaibe force-pushed the feat/coverage-events branch 2 times, most recently from 69366e5 to 0ee8592 Compare December 15, 2018 09:33
Instead of looking at the transaction, instrument the
source to publish coverage event
@alaibe alaibe force-pushed the feat/coverage-events branch from 0ee8592 to d2f27e3 Compare December 16, 2018 10:35
Do not submit event for function and branch
but detect where the statement is
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