Skip to content
This repository was archived by the owner on Jun 29, 2023. It is now read-only.

WIP Add solidity-coverage#552

Draft
jacque006 wants to merge 3 commits intomasterfrom
contracts-test-coverage
Draft

WIP Add solidity-coverage#552
jacque006 wants to merge 3 commits intomasterfrom
contracts-test-coverage

Conversation

@jacque006
Copy link
Collaborator

@jacque006 jacque006 commented Apr 19, 2021

Add coverage for contracts via solidity-coverage to help identify areas that need additional tests both now and in the future.

TODO

@jacque006 jacque006 force-pushed the contracts-test-coverage branch from d36993a to 2f8f036 Compare April 19, 2021 23:07
@jacque006 jacque006 force-pushed the contracts-test-coverage branch from 2f8f036 to f1a69b4 Compare April 19, 2021 23:30
@cgewecke
Copy link

Sorry for the delay debugging this. You're seeing zero-coverage because:

  • solidity-coverage modifies your contracts (injecting trace instrumentation) and recompiles them, saving the result to artifacts
  • you consume your bytecode from typechain factories in types/

You need to run typechain after solidity-coverage executes it's custom compilation. You can do this using a workflow hook in .solcover.js

With this config

const shell = require('shelljs'); // This is a dep at solidity-coverage, no need to install separately

module.exports = {
    skipFiles: ["./test", "./deployment"],
    onCompileComplete: async function(config){
      shell.exec("typechain --target ethers-v5 './artifacts/contracts/**/!(*.dbg).json'");
    }
};

... you'll see:
Screen Shot 2021-04-25 at 3 27 16 PM

@cgewecke
Copy link

Oh! Just remembered, there's (at least) one more potential coverage issue. SC doesn't handle hardhat_reset well, because that method completely re-instantiates the provider.

await ethers.provider.send("hardhat_reset", []);

It should be possible to achieve the same thing using evm_snapshot and evm_revert without running into problems.

@github-actions github-actions bot added client This PR is about implementing the client contracts This PR changes some contracts labels Apr 26, 2021
@jacque006 jacque006 force-pushed the contracts-test-coverage branch from c19952f to b27d37c Compare April 26, 2021 16:34
@github-actions github-actions bot removed contracts This PR changes some contracts client This PR is about implementing the client labels Apr 26, 2021
@jacque006
Copy link
Collaborator Author

Thanks @cgewecke ! Things are looking much better:

Screenshot from 2021-04-26 10-26-46

Still running into some issues with tests involving gas consumption, but I was expecting that from https://github.com/sc-forks/solidity-coverage#usage-notes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants