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

feat: coverage [APE-139] #1423

Merged
merged 105 commits into from
Jul 3, 2023
Merged

feat: coverage [APE-139] #1423

merged 105 commits into from
Jul 3, 2023

Conversation

antazoey
Copy link
Contributor

@antazoey antazoey commented May 1, 2023

What I did

Bulk of the work was done already here: #1337
All this does is add the coverage feature.
It is not too hard to add arcs (branches) next, can do that, but wanted to let this simmer and let the dependent branches merge; it will be easier on me

Fixes: APE-139
fixes: #349

Tests are in ape-vyper: ApeWorX/ape-vyper#84
Also need that PR in order for this to properly work.

How I did it

  • make a pytest-cov plugin

How to verify it

in your favorite project, find your favorite provider with tracing support and do this:

ape test --coverage --network ethereum:local:foundry

you should see something this:

                coverage_test Coverage                 
                                                       
                         Func   Stmts   Miss    Cover  
 ───────────────────────────────────────────────────── 
                  __builtin__       2      0   100.0%  
            _immutable_number       0      0   100.0%  
                      _number       0      0   100.0%  
                 foo_method()       1      0   100.0%  
          foo_method(uint256)       1      0   100.0%  
  foo_method(uint256,uint256)       3      0   100.0%  
                  view_method       1      0   100.0%  

mine is really bad
will update as it moves along

Checklist

  • All changes are completed
  • New test cases have been added
  • Documentation has been updated

@vany365 vany365 changed the title feat: vyper line coverage feat: vyper line coverage [APE-919] May 1, 2023
@antazoey antazoey changed the title feat: vyper line coverage [APE-919] feat: coverage plugi [APE-919] May 1, 2023
@antazoey antazoey changed the title feat: coverage plugi [APE-919] feat: coverage plugin [APE-919] May 1, 2023
@antazoey antazoey changed the title feat: coverage plugin [APE-919] feat: coverage [APE-919] May 1, 2023
@fubuloubu
Copy link
Member

Tested with https://github.com/ApeAcademy/Oracle

Notes:

  1. If I use the default provider (eth-tester) with the --gas flag, it raises an error and say that my provider doesn't support this. However with just --coverage there is no error
  2. I'm not sure I see where the 17 lines it mentions are coming from (are some of these "Virtual" for hidden compiler features?)
  3. I get 0% coverage with my test, which does a fair amount of logic and should easily trigger at least 60%

@antazoey
Copy link
Contributor Author

antazoey commented May 5, 2023

@fubuloubu sorry if i misled but this PR is far from working was moreso showing how to set it up and drive the hooks in.
the actual custom coverage plugin is barely started yet and anything that works is kind of a fluke!

i appreciate you trying it out and i will ping again once it actually does something accurate

fubuloubu
fubuloubu previously approved these changes May 18, 2023
@antazoey
Copy link
Contributor Author

Notes:

  1. If I use the default provider (eth-tester) with the --gas flag, it raises an error and say that my provider doesn't support this. However with just --coverage there is no error
  2. I'm not sure I see where the 17 lines it mentions are coming from (are some of these "Virtual" for hidden compiler features?)
  3. I get 0% coverage with my test, which does a fair amount of logic and should easily trigger at least 60%

i believe all these are resolved now

@antazoey
Copy link
Contributor Author

antazoey commented May 24, 2023

Remaining TODOs for this PR:

  • ensure default arg generated methods work
  • Add exclusion feature, same that we do for --gas. will allow ignoring mocks and debug methods and all that.
  • show methods called
  • allow html gen
  • allow xml gen
  • Document coverage classes more, especially CoverageItem.
  • Increase granularity to factor in source location columns. Right now, only line numbers are factored in and there may be multiple statements on the same line (i did half of this; increased granularity to have column differences available if possible
  • Correct mistakes from new coverage models... before it working but now, we are getting extra stmts
  • Figure out if branch will work with these data structures
  • Calls / View Methods using ape-geth
  • Calls / View Methods using ape-foundry (sorry Hardhat, you need trace call RPC!) (https://github.com/ApeWorX/ape-foundry/pull/53/files)
  • Show auto-getters somehow (only function hit count)
  • try to upload to codecov.io (success:
  • Simple CSS for HTML
  • Have a verbose output (since XML and HTML and more limiting than anticipated)

Questions:

  • is the .build folder a good place to put reports? I could output to look there at the end of the tests.

src/ape/types/trace.py Outdated Show resolved Hide resolved
@vany365 vany365 changed the title feat: coverage [APE-919] feat: coverage [APE-139] Jun 2, 2023
@antazoey
Copy link
Contributor Author

antazoey commented Jun 5, 2023

XML example (this gets automatically exported to .build)

<?xml version="1.0" ?>
<coverage version="0.6.11" timestamp="1686262016035" lines-valid="13" lines-covered="7" line-rate="0.5385" branches-covered="0" branches-valid="0" branch-rate="0" complexity="0">
  <!-- Generated by Ape Framework: https://docs.apeworx.io/ape/stable/index.html-->
  <!-- Based on  https://raw.githubusercontent.com/cobertura/web/master/htdocs/xml/coverage-04.dtd -->
  <sources>
    <source>/Users/jules/ApeProjects/ape-playground/contracts</source>
  </sources>
  <packages>
    <package name="." line-rate="0.5385" branch-rate="0" complexity="0">
      <classes>
        <class name="Contract" line-rate="1.0" branch-rate="0" complexity="0">
          <lines>
            <line number="6" hits="2"/>
            <line number="7" hits="4"/>
            <line number="8" hits="3"/>
            <line number="9" hits="3"/>
            <line number="15" hits="1"/>
          </lines>
        </class>
        <class name="IgnoreSomeOfMe" line-rate="0.0" branch-rate="0" complexity="0">
          <lines>
            <line number="4" hits="0"/>
            <line number="5" hits="0"/>
            <line number="6" hits="0"/>
          </lines>
        </class>
      </classes>
    </package>
  </packages>
</coverage>

Edited: we are now using the schema per spec code codecov

src/ape/types/coverage.py Outdated Show resolved Hide resolved
src/ape/types/coverage.py Outdated Show resolved Hide resolved
src/ape/types/coverage.py Show resolved Hide resolved
src/ape/types/coverage.py Outdated Show resolved Hide resolved
@fubuloubu
Copy link
Member

Random question: would it be possible to combine these with reports in .coverage for normal python code? For some SDKs that would be a nice feature

@antazoey
Copy link
Contributor Author

antazoey commented Jun 7, 2023

Random question: would it be possible to combine these with reports in .coverage for normal python code? For some SDKs that would be a nice feature

That would be a nice feature.

codecov.io is a service that does this too.

Since ive been mostly copying how Python does it, I think we could both have a merge function as well as codeov support. For sure the second one, I am going to make a TODO for that.

@fubuloubu
Copy link
Member

Random question: would it be possible to combine these with reports in .coverage for normal python code? For some SDKs that would be a nice feature

That would be a nice feature.

codecov.io is a service that does this too.

Since ive been mostly copying how Python does it, I think we could both have a merge function as well as codeov support. For sure the second one, I am going to make a TODO for that.

Sweet! Doesn't have to be in this PR, but glad you agree it would be really awesome to support

@antazoey
Copy link
Contributor Author

antazoey commented Jun 7, 2023

btw I really tried to use pytest-cov and coverage.py more but ran into a lot of issues. They are analyzing C stackframes and stuff and updating databases really fast, hard to really play with. They have a way to add tracers for non-python types , provided those files show in the python traceback. In Ape, we do inject revert lines in the python traceback that point to the contract, but not the entire trace.. that would be crazy... so I ended up ditching that plan and instead "being inspired by coverage.py and pytest-cov".

@antazoey
Copy link
Contributor Author

antazoey commented Jun 7, 2023

we should make the HTML prettier tho, if i have time ill add a small CSS but we could add some branding and show off alex here eventually

fubuloubu
fubuloubu previously approved these changes Jul 3, 2023
@dtdang
Copy link
Contributor

dtdang commented Jul 3, 2023

For the html report, would it also be beneficial to have the verbose option to be built out on there or would that not really be necessary?

dtdang
dtdang previously approved these changes Jul 3, 2023
@z80dev
Copy link
Contributor

z80dev commented Jul 3, 2023

Tested and worked great for me, left a minor comment previously

@antazoey antazoey dismissed stale reviews from dtdang and fubuloubu via b32bebc July 3, 2023 23:05
@antazoey
Copy link
Contributor Author

antazoey commented Jul 3, 2023

For the html report, would it also be beneficial to have the verbose option to be built out on there or would that not really be necessary?

ya i was thinking about this... I know Alex is helping improve the HTML, probably a good refactor / #GoodFirstIssue kind of situation when it comes to improving the HTML reporting in general; the sky is kind of the limit there.

However, to address this comment, I went ahead and added some verbose HTML support so you can at least get verbose tables similar to the terminal output.

Thank you all for reviewing!

dtdang
dtdang previously approved these changes Jul 3, 2023
@antazoey antazoey merged commit 44190ab into ApeWorX:main Jul 3, 2023
15 checks passed
@antazoey antazoey deleted the feat/cv branch July 3, 2023 23:41
@ControlCplusControlV
Copy link

🍾

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.

Testing: Test coverage w/ code-cov support
5 participants