sc-forks / solidity-coverage

Code coverage for Solidity smart-contracts
MIT License
982 stars 267 forks source link

Test installation on some larger projects #10

Closed cgewecke closed 7 years ago

cgewecke commented 7 years ago

And possibly PR these in? If useful?

cgewecke commented 7 years ago

In principle, tool will run on melonport. Report looks like this. Truffle / Solidity-Parser are having a problem with some stub contracts they have, but otherwise everything works.

Chronobank - which had tests failing under coverage because there was no events filtering also works. They're pushing lots of code to a development branch right now though . . .

cgewecke commented 7 years ago

Numerai report looks like this

They had a couple issues worth pursuing. This line in their contracts throws a solidity-parser error while instrumenting (but not when compiling with Truffle). Putting the return statement on the same line as the function call resolved it but this may be an SP regression bug. The fork we use is several commits ahead of the published version Truffle uses.

And some of their minting tests fail non-deterministically under coverage - these fire events. This appears to be a timing related bug (so, basically bad news).

cgewecke commented 7 years ago

Bancor @ commit 6e0f2d2 : Lots of tests but not running clear ATM . . . . holding off on this.

cgewecke commented 7 years ago

Aragon has a lot of hardcoded gas values - I tried replacing those with much higher sends, and also tried funding accounts with tons of gas. But it still exits with out of gas when deploying the contracts. Just don't know what to do . . . . Possibly revisit. FWIW everything seems to instrument and compile correctly.

UPDATE Works: Issue was hardcoded gas values in the migrations. Tests run clean and generate this report.

cgewecke commented 7 years ago

0xProject solved by @maurelian.

maurelian commented 7 years ago

Changes made for 0x:

cgewecke commented 7 years ago

Closing - no longer really doing this kind of validation.