JoinColony / solcover

Code coverage for solidity
MIT License
64 stars 8 forks source link

Uncovered lines in instrumentSolidity.js #43

Closed cgewecke closed 7 years ago

cgewecke commented 7 years ago

This is what instrumentSolidity.js's coverage report looks like after adding unit tests for conditionals. There are few lines that aren't being run that may be difficult to hit.

(NB: This issue is book-keeping for changes in the conditional-unit-tests branch which modifies the parsing slightly. The above not necessarily true of master at present.)

cgewecke commented 7 years ago

Edit: qualified the above to about conditional-unit-tests branch.

area commented 7 years ago

The line x+2 would get hit by BinaryExpression, though obviously that doesn't do anything, and we don't do anything with the instrumentation there... even so, it's valid Solidity, so we should probably support it even if nothing happens there, and check that we do correctly!

New Contract(args) can be on a line by itself. You lose the address of the contract, obviously, but is there a reason you'd do stuff in the constructor of a contract and then selfdestruct at the end of the constructor? This would be a very edge case though! Again though, it's technically valid, so we at least need the function there and can test for it.

You're right. closeBracketStart is already not used anywhere, and the only branch that uses closeBracketEnd is only hit (I believe) when instrumenting the contract before the preprocessor has run on it, so we can remove that too.

I've made a couple of commits that deal with these situations.

area commented 7 years ago

And now rebased, so a PR should work properly 👍

cgewecke commented 7 years ago

Cool!