Comments (33)
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
This issue now has a funding of 20000.0 DAI (20000.0 USD @ $1.0/DAI) attached to it as part of the synapsecns fund.
- If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
- Want to chip in? Add your own contribution here.
- Questions? Checkout Gitcoin Help or the Gitcoin's Discord
- $4,415,122.51 more funded OSS Work available on the Gitcoin Issue Explorer
from foundry.
One thing to note is when improving fuzzing as part of #387, we may want to hook into the coverage results to enable coverage-guided fuzzing, i.e. the fuzzer would know what parts of code have or have not been covered and use that to guide input mutations. I'm not familiar enough with the internals to know if the above plan enables that but just wanted to mention it in case it affects coverage implementation
from foundry.
Coverage should generate LCOV files at the very least so it is uploadable to tools like CodeCov. The LCOV file can also be used to generate the HTML reports and so on, so really, we "only" need the LCOV file and the rest can be built on top.
For coverage thresholds, I think people mostly defer to other tools that also take LCOV?
from foundry.
So, as promised, here is a status update.
I was already working on this issue prior to the bounty (which is why I am assigned).
Initially, the thought was to analyse JUMPI
and JUMPDEST
s to find branches (i.e. require
, assert
, if
etc.), however it would not get us all the way in terms of the information we want - we want functions, branches, executable lines and statements.
Background
The primary mode of operation for the coverage tool will be to collect the following pieces of information:
- Functions
- Executable lines: lines, excluding lines that do nothing (whitespace, comments, brackets on single lines etc.)
- Branches: If statements, calls to require and asserts
- Statements: executable statements, which differ from lines in that you can have multiple statements per executable line.
This information will be collected from the AST of the Solidity files. Then the tests are run with a special coverage collector that marks opcodes as hit as they are executed, which we later use (along with source maps) to map back to the coverage items.
Report types
The above information will be output either to stdout as a simple report, or as an LCOV file that you can upload to services like CodeCov. A stretch goal would be to add a HTML report as well, but since tools already exist for converting LCOV to HTML, it isn't a deal breaker if it is not in the first version.
Compared to other coverage tools
- DappTools's coverage tool is not super great (see comments above) as it does not support outputting to standard reporting formats like LCOV, and it does not treat
require
andassert
as branches. - Hardhat Coverage is closer to what we want, but it's slow and it distorts gas since it instruments the code of the contracts
Current status
I went with the jump analysis first, but as I found it insufficient, I had to scrap most of my work and start over with the method described above. The coverage collector is done, but I am currently having some issues constructing the higher level coverage data that the reports will use (see below)
Current issues
- The AST implementation in
ethers-solc
is not entirely complete yet, so I am implementing nodes as I go - The AST implementation in
ethers-solc
only works for Solidity >=0.8.0 currently since the AST had a breaking change there, so we need to add support for <=0.8.0 but it is not a blocker since this work can be done in parallel - Source maps in Solidity identify source files with a source ID. This source ID is required to be unique, and it is if you compile all files in one go, but
ethers-solc
currently tries to compile multiple files in parallel, leading to source ID clashes. This is a blocker, since we need to be able to map bytecode of any contract to the coverage items using the source map.
from foundry.
Sorry guys, this is taking longer than expected. I've opened up a draft PR with a status update, and I intend to update that PR as I go with new status updates if relevant. #1576
from foundry.
Notes on how we can achieve coverage:
- We parse the source files using
solang-parser
and take a note of where we want to check for coverage in three buckets: branches, functions and lines. These ranges are saved somewhere so we can reference them later (solidity-coverage
has a file with the relevant AST nodes in it we can reference) - We hook into
Handler::pre_validate
in Sputnik, lookup where we are in the source and check if that is a range we are watching. If it is we increment the visited count for that piece of code by 1 (regardless of if it's a branch, function or line). - Each test generates its own coverage report. At the end of testing, we merge them
- This coverage report should be easily converted into at least the lcov format, since we can use that to either: dump it in a file, generate a HTML report or print status on the CLI.
Points where I'm stuck:
- I'm not sure how to get the source map in the
MultiContractRunnerBuilder
. It seems like I somehow need to switch the artifact output type? - Generally, is there any real reason we have different representations of the artifacts as a consumer of the
ethers-solc
package as opposed to a generalized struct? - It seems like the debugger used a manual compile step and I think if we need to do that for coverage too we should consider at least some sort of refactoring so we don't end up having a lot of different compile pipelines scattered around as it is hard to maintain
from foundry.
Just want to add a strong vote for this feature. For context, our monorepo relies heavily on solidity coverage, and CI checks will fail when a change would reduce coverage below a certain threshold.
from foundry.
UX Proposal
This is largely based on how Istanbul's JS coverage reports work, since that's what I've used the most and haven't had any big issues with it's approach/formatting (whereas the dapptools format is clunky and not great IMO). Istanbul is what Hardhat's coverage tool uses to generate output data/reports.
As a result, this initial proposal pretty much just describes how Istanbul works, but happy to iterate and change things based on feedback.
Standard Syntax
Coverage should not be the default since the added instrumentation will likely significantly slow down tests, so instead we should run coverage with:
forge test --coverage
It would be great if this also supported watch mode and the various match flags, so you can do
forge test --coverage -w --match-contract ContractWereIncreasingCoverageFor
CI Syntax
I'd be curious to hear more from @maurelian about how this should work, but a CI mode would be great so you can configure your CI to fail if aggregate coverage drops below some threshold. One possible syntax is instead of running the above command you'd run:
forge test --coverage-threshold 95
This will cause the process to exit with an error if coverage is < 95%
Terminal Output
At the bottom of the terminal output, append a table summary of coverage to the standard test output. Something like the below code block, where >= 80% coverage is colored green, 65% <= x < 80% coverage is yellow, and < 65% coverage is red (I believe these are the thresholds used by Istanbul).
$ forge test --coverage
[⠆] Compiling...
No files changed, compilation skipped
Running 1 test for src/test/MyContract.t.sol:MyContractConstructor
[PASS] testConstructor() (gas: 31072)
Test result: ok. 1 passed; 0 failed; finished in 10.32ms
-------------------|-----------|-----------|-----------|-----------|
File | % Stmts |% Branches | % Funcs | % Lines |
-------------------|-----------|-----------|-----------|-----------|
src/ | 100 | 100 | 100 | 100 |
MyContract.sol | 100 | 100 | 100 | 100 |
-------------------|-----------|-----------|-----------|-----------|
All files | 100 | 100 | 100 | 100 |
-------------------|-----------|-----------|-----------|-----------|
=============================== Coverage summary ===============================
Statements : 100% ( 350/350 )
Branches : 100% ( 176/176 )
Functions : 100% ( 81/81 )
Lines : 100% ( 317/317 )
================================================================================
File Output
In addition to the terminal output, output files containing more details should be saved to a coverage
folder. Istanbul saves this as HTML/JS/CSS files where:
- The top level HTML file is a similar summary to the terminal output
- You can click into each file to see summary tables for each folder/subfolder, then can click on specific files for coverage details about that file.
- The JS files are so you can sort tables and navigate the pages more easily.
- The CSS files are unsurprisingly for styling the report.
The syntax used by Istanbul in these reports can be found here. To summarize:
- The following coverage criteria are reported: function coverage, statement coverage, branch coverage, and line coverage.
- When viewing source code in an HTML file, the lines are marked with
E
orI
to specify whether the else path or if path was not taken. - A number such as 291x indicates the line was executed 291x times.
- Statements, functions, branches, and lines that were not covered are colored pink, orange, yellow, and red, respectively
Screenshots from random reports I found online are below:
from foundry.
I also think forge coverage
makes more sense to not clutter the interface of forge test
with even more options. For example, we want at least --coverage-threshold
(or just --threshold
if on forge coverage
), report types (--report <type>
) and more.
There are also some clashes between forge test
and the coverage behaviour; for example, we can't run coverage with optimizations turned on, but since you could conceivably do forge test --gas-report --coverage
this may lead to "bug reports" even though it is intentional, since it might be unexpected behavior
from foundry.
sadly theirs is pretty bad tbh, it doesn't have any branching info for == statements, etc
from foundry.
Running it with the optimizer off makes sense 👍
For the coverage; yes LCOV format would be good. The Go description was just an example of the how the output might be used in editors. As long as forge
can output the LCOV file to a specified location, I should be able to integrate it with editor plugins relatively easily.
from foundry.
forge coverage
feels more consistent to me with the existence of forge snapshot
.
from foundry.
Reopening this - still some edge cases and missing stuff. See this label:
Cmd-forge-coverage
from foundry.
I'd be curious to hear more from @maurelian about how this should work, but a CI mode would be great so you can configure your CI to fail if aggregate coverage drops below some threshold.
Yeah, I can't find it now that we've migrated from GHA to Circle CI, but we previously dependedon tooling from CodeCov to cause the CI failure.
One thing I forgot to mention is how fuzz/invariant tests should affect coverage reports. I think there's a few ways we can handle this.
When I think of coverage I think of unit tests, and I'd want to be able to use the report to direct me to where further tests are needed. So, my default is to not mix the two kinds of coverage up, at most have a separate report for each.
from foundry.
Let's leave this open for now, eventually all coverage stuff will be aggregated into #4442 and I want to make sure things in here don't get missed
from foundry.
can you recommend an algorithm, and we can start scoping it out along with how we'd utilize sourcemaps?
from foundry.
only good one i know is solidity-coverage's but it works by instrumenting the solidity whereas we'd probably want to do it at the VM level?
from foundry.
Yes, I didn't love the injected events approach...what is the hevm approach missing? is it hard?
from foundry.
More notes:
- What's missing: HEVM approach does not account for branching in logical statements (||, <, &&, >, ==, etc) in requires (and in other places?)
- HEVM approach is this: On each step, look up where we are in the bytecode, then map that onto the relevant line in the source code and increment the "visited" counter on that line by 1. Any line that has been visited at least once is considered covered
- Solidity Coverage approach: Parsing the contract into an AST, walking the AST and finding points of interest and then injecting function calls or events they can hook into later
- We should generate the coverage report in two formats: in-line text report for the CLI and an LCOV file for CI purposes/additional tooling
- We should probably investigate doing the HEVM approach but making up for their shortcomings (i.e. only do the solidity-coverage emit-events approach as a last resort)
from foundry.
@maurelian I'm planning to post an initial UX proposal for coverage later today (though deferring to @gakonst on actual implementation timelines), which will be based on my experience with dapptools + Instanbul style coverage reports, since those are the main coverage tools I've used. If you have any UX suggestions or tools who's reports/workflows you like/can link to, that would be great!
from foundry.
ACK - coverage is next up on our prio list.
from foundry.
Coverage should generate LCOV files at the very least so it is uploadable to tools like CodeCov. The LCOV file can also be used to generate the HTML reports and so on, so really, we "only" need the LCOV file and the rest can be built on top.
Ah perfect that makes sense, thanks.
For coverage thresholds, I think people mostly defer to other tools that also take LCOV?
Seems this is what Optimism does. Though is there a reason we wouldn't want to integrate at least a simple coverage threshold natively so it can be used to trigger a CI failure? Seems nice to not need a third-party tool, and it'd facilitate having a better forge init
out of the box. But I haven't used Codecov or other similar tools, so I'm not sure what nice-to-have features they may have that a native approach won't have.
One thing I forgot to mention is how fuzz/invariant tests should affect coverage reports. I think there's a few ways we can handle this:
- Ignore fuzz/invariant tests from coverage reports so reports are deterministic.
- Include them, and settle for non-deterministic coverage. This is arguably ok since it means you may be relying too heavily on broad fuzz tests and not enough targeted unit tests?
- Include fuzz runs, but only after implementing #991 so coverage can be deterministic.
- Add a flag so the user can determine whether or not fuzz test runs should be included in coverage reports.
from foundry.
When I think of coverage I think of unit tests, and I'd want to be able to use the report to direct me to where further tests are needed. So, my default is to not mix the two kinds of coverage up, at most have a separate report for each.
Hmm, interesting! I find sometimes I only write fuzz tests for certain portions of code, since having a separate concrete test for the same code is redundant. But I see your use case, so perhaps an --include-fuzz
flag is the best option here. And if you always run forge coverage --include-fuzz
, but see a regression in coverage, that's an indication you probably should e.g. add a concrete test to ensure consistent coverage.
(Side note: Above I suggested forge test --coverage
, but when writing this I naturally went to forge coverage
. I think either is fine fwiw)
from foundry.
Hello, I'm very interested in helping out with this feature! I posted some discussion here #1348 before I realised this issue existed.
Initial thoughts after catching up:
- Are we settled on the implementation at the solidity level via AST manipulation rather than at the VM level?
- I'm definitely in favour of the tooling just producing a standardised format and leaving the reporting/html generation/etc to other tools. There are many of them which already exist. Does rust have a native coverage format? Ideally using a format which doesn't require another package manager (e.g. npm) to install dev tooling would be nice.
- I would personally include fuzzing in the coverage but I can see the argument for a flag. Would prefer if the flag was opt-out though.
- I would lean towards
forge test --cover[age]
because all the same options (e.g.--match
) will be useful/needed as well.forge cover
could be an alias forforge test --cover
.
And finally, the gold standard here would be to integrate the coverage report with the solidity plugins for editors, so that you can see the coverage results directly in your editor every time you save. This massively plays to forge
's strength as tests are feasible to run on save, which isn't really possible with other environments.
The Go plugin does this and it makes the feedback loop for tests incredibly fast
I am completely new to the codebase but keen to help out. I think the best way to parallelise would be to try and understand any dependencies or refactors needed and possibly try to create some distinct issues 🙏
from foundry.
Are we settled on the implementation at the solidity level via AST manipulation rather than at the VM level?
I think the current thinking is that we would not use the AST. We would instead on a VM-level check jump instructions and map those to the source code using source maps, but the approach hasn't been validated so that might change.
And finally, the gold standard here would be to integrate the coverage report with the solidity plugins for editors, so that you can see the coverage results directly in your editor every time you save. This massively plays to forge's strength as tests are feasible to run on save, which isn't really possible with other environments.
Do you have any additional info on how this works? I'd assume it uses LCOV - if that's the case, then integration should be simple. We probably just need a way to output the raw LCOV to stdout
from foundry.
I think the current thinking is that we would not use the AST. We would instead on a VM-level check jump instructions and map those to the source code using source maps, but the approach hasn't been validated so that might change.
Ah right, thanks. I had a question on this; if the compiler makes optimisations like removing branches, can we still map the bytecode back to source lines? Users clearly operate on the source-line level, so seeing missing coverage on a line because of something the compiler does would be confusing. I've seen that some languages seem to implement coverage with LLVM so I assume this is a solved problem, but not sure how it works.
Do you have any additional info on how this works? I'd assume it uses LCOV - if that's the case, then integration should be simple. We probably just need a way to output the raw LCOV to stdout
Go has its own native coverage format (which is an extremely simple text file). The editor runs the tests and the file is output to a tmp directory, which is then read, parsed and rendered in the editor. I'd be happy to try and take this part on because I might be more use there, as I'm very unfamiliar with the vm code and rust.
from foundry.
Ah right, thanks. I had a question on this; if the compiler makes optimisations like removing branches, can we still map the bytecode back to source lines? Users clearly operate on the source-line level, so seeing missing coverage on a line because of something the compiler does would be confusing. I've seen that some languages seem to implement coverage with LLVM so I assume this is a solved problem, but not sure how it works.
Probably not. The source maps in Solidity are really flaky when you turn the optimizer on, so we would probably need the optimizer to be off. I'm not sure, though, I'll figure that out as I implement the feature
Go has its own native coverage format (which is an extremely simple text file). The editor runs the tests and the file is output to a tmp directory, which is then read, parsed and rendered in the editor. I'd be happy to try and take this part on because I might be more use there, as I'm very unfamiliar with the vm code and rust.
Ah, I misunderstood as well - you don't want Forge to necessarily support Go's coverage format, just a coverage format that can be used in editors? If so, I think most editors should be able to support LCOV (used by C and others), or at least something proximal to it
from foundry.
Trying to summarize the TODOs
- add
forge coverage
- add
--threshold
tocoverage
to enforce minimal coverage - add
--include-fuzz
tocoverage
(or --exclude-fuzz if we chose opt-out) - parse sources with
solang-parser
and creates buckets - hook into Sputnik
Handler::pre_validate
and increment visited count - generate each test coverage report
- merge all tests reports
- generate LCOV file
- generate terminal output (from LCOV ?)
Anyone already started working on this ?
I'm down to work on it but may lack some knowledge on the VM side
from foundry.
Yes, I am actively working on it. Will post a status update tomorrow (I am not home)
The todo list is no longer valid since that was based on Sputnik, and we've since moved to REVM :)
from foundry.
Issue Status: 1. Open 2. Started 3. Submitted 4. Done
Work has been started.
These users each claimed they can complete the work by 264 years, 6 months from now.
Please review their action plans below:
1) bastek8 has applied to start work (Funders only: approve worker | reject worker).
Program i wszystlo z hun zwiazane
2) onbjerg has been approved to start work.
I was already working on this feature before the bounty was added
3) malavekevin36 has applied to start work (Funders only: approve worker | reject worker).
I work plan and whoever don’t want to come back to work they can find a new job
Learn more on the Gitcoin Issue Details page.
from foundry.
May I ask what the status is with coverage generation of fuzz tests? It looks to me as though they are not yet included and I don't yet see an --include-fuzz
flag as discussed above.
from foundry.
Close-able? @mds1
from foundry.
I have a question. I still see branch coverage misses for assertions. E.g.
I don't think it is ideal. But what is the expected behavior as of now?
from foundry.
Related Issues (20)
- debugger: out of bounds crash
- Arbitrum fork test keeps failing when calling `arbBlockNumber()` HOT 1
- Cast rpc eth_gasPrice --raw returns code: -32600, message: Invalid request HOT 1
- remappings.txt does not work with forge build. HOT 1
- Cast gives ambiguous error on invalid RPC connection HOT 1
- Invariant tests: `preserve_state` is not respected when `fail_on_revert = false` HOT 2
- Invariant BSC fork test fails: "fuzz: failed to inspect: Database error: failed to get account for 0x0000000000000000000000000000000000001d2F: Deserialization error: EOF while parsing a value at line 1 column 0"
- In-line test configuration not respected HOT 1
- `forge build` panics if its cwd is deleted
- forge test extremely low when stack trace is enable (-vvv) HOT 2
- Gas report for fuzz test and internal transactions: confusing output
- Delete does'nt work great with chisel mappings HOT 1
- `forge script` should use sender address of the provided keystore HOT 3
- [cheatcodes] Add option in `recordLogs` to filter by topic and/or emitter address
- Tracking: improve handling and correct prioritisation in CLI HOT 1
- Non-stable cheatcodes should emit warnings and errors
- Script Periodically Forgets Remappings HOT 2
- forge(doc): @inheritdoc does not pickup description HOT 9
- Pin alloy deps to a specific hash HOT 2
- Issues with trezors
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from foundry.