Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration to 0x coverage tools #1699

Closed
wants to merge 3 commits into from
Closed

Migration to 0x coverage tools #1699

wants to merge 3 commits into from

Conversation

nventuro
Copy link
Contributor

@nventuro nventuro commented Mar 29, 2019

For a while now we've been having multiple issues with solidity-coverage. A bug I encountered yesterday broke the proverbial camel's back: when restoring a snapshot, account nonces are not restored, which causes issues with some deploy schemes, such as ERC1820's.

This is an effort to migrate our testing infrastructure to 0x's sol-coverage, which will hopefully be more stable. Many changes needed to be made however, since it uses web3-provider-engine under the hood. I also had to move some contracts around, since 0x's tooling requires each contract to be stored in a file named after it.

@nventuro nventuro added improvement automation Tests and coverage running. Docsite publishing. labels Mar 29, 2019
@nventuro nventuro requested review from frangio and removed request for frangio March 29, 2019 19:46
@nventuro
Copy link
Contributor Author

This is still WIP, but I want to take a look at how long this new setup takes on Travis.

@frangio
Copy link
Contributor

frangio commented Apr 6, 2019

As of #1684 (comment), we were able to update our current coverage analysis tool to solve the bug we had ran into. It'll last a bit longer like that, but solidity-coverage being unmaintained, it will break again.

Our experience trying out @0x/sol-coverage was also kind of rough unfortunately. @nventuro can share more details about it.

@nventuro
Copy link
Contributor Author

nventuro commented Apr 8, 2019

Most of the issues come from the fact that 0x's tooling uses web3-provider-engine, so in order to connect to a ganache node you either:
a) setup a GanacheSubprovider
b) connect to it via the RPCSubprovider

As can be seen in the test runs in this PR, option a is very slow (between 2 and 3 times slower than our current setup). It looks like the suite becomes slower as each test is executed, since running single contracts with .only takes about the same on both setups: maybe there's some sort of resource leakage going on.

Option b had other problems, and in the end we couldn't get it to work. There may be a bug somewhere in the stack, since after a couple requests the provider enters a loop where it asks for the receipt for a block that hasn't been mined yet, which it never exits.

Even if we managed to have the tests running, the coverage tools still lack some features solidity-coverage has (such as instrumenting contract creations from inside another contract, testing both branches of a require, among others). Because of this, we'll be sticking with it for now.

@nventuro nventuro closed this Apr 8, 2019
@nventuro nventuro deleted the 0x-coverage branch April 8, 2019 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation Tests and coverage running. Docsite publishing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants