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

Do not require tests to use .call() #159

Merged
merged 1 commit into from
Nov 25, 2017
Merged

Do not require tests to use .call() #159

merged 1 commit into from
Nov 25, 2017

Conversation

area
Copy link
Contributor

@area area commented Nov 25, 2017

See #146

@area
Copy link
Contributor Author

area commented Nov 25, 2017

Urgh, yeah, that might even be why those tests aren't passing.

@codecov-io
Copy link

Codecov Report

Merging #159 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #159      +/-   ##
==========================================
+ Coverage   97.09%   97.14%   +0.04%     
==========================================
  Files           6        6              
  Lines         379      385       +6     
  Branches       86       88       +2     
==========================================
+ Hits          368      374       +6     
  Misses         11       11
Impacted Files Coverage Δ
lib/parse.js 94.11% <100%> (+0.24%) ⬆️
lib/instrumentSolidity.js 100% <100%> (ø) ⬆️
lib/coverageMap.js 100% <100%> (ø) ⬆️

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 7ae7f54...986eead. Read the comment docs.

@cgewecke cgewecke merged commit 92b3db2 into master Nov 25, 2017
@cgewecke
Copy link
Member

Fantastic, thanks for doing this. :)

@cgewecke
Copy link
Member

cgewecke commented Nov 25, 2017

Unfortunately it looks like we might need to take a more brute force approach than this PR. Zeppelin executes most of their tests through .sol mocks that are stored in their tests folder. Those mocks are not part of the coverage measurements so they don't get swept during instrumentation.

Travis run for their latest shows the problem.

Another strategy...

  • compile before instrumentation
  • load the original abis
  • instrument
  • compile instrumented contracts
  • swap original abis into instrumented artifacts
  • test

Going to take a stab at this tomorrow.

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