Comments (9)
Here's a half-baked idea that might make things easier:
PreAssertEvent();
assert(1==2);
PostAssertEvent();
If nPreAssertEvents==nPostAssertEvents
, then the branch where the assert fails is missing, and if nPostAssertEvents==0
then the branch where the assert succeeds is missing.
from solidity-coverage.
So my rough draft of what needs doing is now on the feature/assert-require-branches
branch. The test for a failing assert currently doesn't work, because we're using the 'standard' etheruemjs-vm
in our tests, and so when the assert fails, all changes are reverted, and we don't have any events!
I'll come back to this tomorrow, but if you wanted to throw some tests in there to break what I've already done, I'd definitely welcome that - and any other comments you might have are more than welcome, too!
from solidity-coverage.
for require(xyz)
if(!(xyz)){ assembly{ jump(invalidJumpLabel) } }
Assembly will stop solidity from warning about throw? And how should highlighting be done?
from solidity-coverage.
Oh-ho! Very nice spot. We should consider that valid solidity includes:
function test(){
var () = assert(1==1);
}
function test2(){
assert(
return1()
!=
return2()
);
}
The first is very unlikely to be seen, but I could believe people would use something like the second.
In theory, the general solution of turning these in to conditionals works. It's going to require some extra magic floating around though, I think. Currently, the source mapping we generate comes from the original source file, which doesn't recognise these as conditionals; as a result, all conditional ids in the map are going to be wrong.
I tried typing up what I think the procedure should be here, but I kept finding problems with each of my attempts. I'll keep thinking!
In terms of highlighting, I assume it would be the same as how we show something like:
if (x==true) { doThing() }
i.e. show the 'E' in the margin indicating that the 'Else' branch hasn't been run, even though that branch isn't 'in' the source code? There would need to be a bit added to the README to explain why assert
s were being counted as branches, but I do think this is justified.
from solidity-coverage.
The events strategy is a really clever way of detecting here. I have implementation questions:
-
With this approach how will we include
require
andassert
statements in the branch coverage if we're detecting this behavior at the end of the run? We should also remember to add those events to the filter file in order to keep event pollution down. -
If (1) is tricky, what are the consequences of pre-processing these statements before we generate the initial mapping? There might be a way of writing the conditional so that it encoded position information that was detectable at
instrumenter.js
. i.e we looked into the consequent block statement to see variables calledsc_implicit_conditional_start = startColVal
etc, or something. And then coerced the necessary values there? We would also need to not parse the body of that conditional? Not sure this idea is coherent or correct . . . -
In the multiline function calls edge case above, do you feel those should be getting line coverage? Other coverage?
from solidity-coverage.
Also do you want to be assigned this? You don't have to take it (but you do have to figure out how to do it, sorry).
from solidity-coverage.
If Istanbul doesn't make a fuss about the ordering of the branchIds (i.e. branch with id 6
is allowed to be earlier in the source code than branch with id 5
), then we could plausibly add on to the branchMap
object at the end of the run... maybe 😄 . I need to remind myself of so many things...!
I definitely want to try this approach before experimenting with pre-processing, though, which I think could easily get out of hand from an implementation perspective quite quickly.
Yeah, I'll take this. Given this won't rely on npm
stuff, I'm much happier with this sort of thing.
Oh, my kingdom for a ,
operator...
from solidity-coverage.
Ok great! I'll happily write tests for this too, don't worry about that unless you it's helpful to you to do them.
from solidity-coverage.
LGTM! Beautiful.
Pushed to that branch:
- Added vm-sc to the dev dependencies (it's about to get swallowed by webpack)
- Made the test vm read from
allFiredEvents
- Fixed old function tests that we're passing but shouldn't have been.
- Added code to ignore solc warnings
- Removed the
process.env
test filter - Strengthened events filter test to include assertion events and conditional events.
- Added gratuitous multiline require statement test.
Everything's passing! I vote merge.
from solidity-coverage.
Related Issues (20)
- Get different hashes between `npx hardhat test` and `npx hardhat coverage` HOT 1
- Consider using caret range for dependencies HOT 3
- named mapping keys printing large logs HOT 3
- Coverage overwrites provider logic in extendEnvironment hook HOT 9
- uncovered else statements HOT 1
- ParserError solidity-coverage: v0.8.5 Solidity 0.8.19 HOT 3
- modifierWhitelist causes inaccurate coverage HOT 3
- Specify minimum coverage HOT 1
- v0.9.0 Road Map
- Donating Funds to Solidity-coverage HOT 2
- ...
- Update tests
- Running solidity-coverage against a generic RPC node HOT 12
- Small docs Improvements
- Unskip stack-too-deep unit test HOT 1
- Try/Catch should be treated as branch
- 0% coverage when using hardhat-foundry & foundry.toml is present HOT 15
- Add tests for file level function declarations
- feature: track config state in Hardhat cache
- Tracking Issue: using viaIR in versions >= 0.8.7 HOT 2
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 solidity-coverage.