Allow files to be skipped during coverage #52
Conversation
While ordinarily we shouldn't want to do these, it is possible to construct valid contracts using assembly that break when the coverage events are injected.
Codecov Report
@@ Coverage Diff @@
## master #52 +/- ##
=======================================
Coverage 97.57% 97.57%
=======================================
Files 6 6
Lines 330 330
Branches 78 78
=======================================
Hits 322 322
Misses 8 8Continue to review full report at Codecov.
|
|
LOL no I'm completely ambivalent about the comments thing - I just saw that Istanbul's taking that approach and ESLint as well and was wondering whether that's becoming normative or something. Good point re: libs. Your analysis of the path forward is fantastic too! It's a complete rewrite but it would be so much more robust and closer to solc / Solidity. And cooler. (I see that while writing this note coulter merged that Too Big PR! Ha ha!) Great. As far as V1 - I think this is very close to being finished. Shouldn't this go back over to Colony and publish it over there? Is there anything else you want to do here? Open to anything but that seems like it makes sense to me. |
Got to dash now, but I'll get back to you tomorrow! |
|
👍 |
|
Published w/ 0.1.6 |
While ordinarily we shouldn't want to do this, it is possible to construct valid contracts using assembly that break when the coverage events are injected.
An example of such a contract is the EtherRouter contract by Peter Borah. It repeatedly calls
mload(0x40)when it needs a memory address, as it only uses one piece of memory at a time and repeatedly overwrites the memory starting at the address contained in0x40. In Solidity,0x40is a special memory address that contains the address of the first empty slot. When events in solidity are compiled, they use the address contained in0x40and update it, and so the injected instrumentation events clobber the memory thatEtherRouteris using.This time, I've also included a test to demonstrate that skipping a contract works!
Really, this is just another sign pointing towards a huge do-over for
solidity-coveragethat I've been trying not to think about. Oncetestrpcallows debug_traceTransaction, we should really be using that functionality plus source mappings to generate coverage. We would no longer require a customtestrpc, wouldn't have to have to make our own coverage environment, no need for custom very high gas limits, or run in to these sorts of issues where instrumentation breaks contracts...EDIT: I am a terrible collaborator. Just noticed #17 existed, and you wanted to do it a different way... I don't think that using comments is the way to go though. In some cases, I would expect the contracts that wanted to be ignored would be libraries that the user will not want to edit directly.