Support for multiple truffle configs compilation

[elenadimitrova]

When running coverage against a repo with a number of different truffle configs (we have nine https://github.com/argentlabs/argent-contracts/pull/146) and a custom compilation setup, solidity-coverage fails to compile correctly as it tries to compile the entire contracts folder assuming all contracts inside are compiled with the root truffle-config.js. This may not always be the case and even though I can probably preemptively compile the project in the onServerReady hook, the compilation will still be triggered after that and it will always fail. I'm open to suggestions as to what can work best.

I also have a problem with the root truffle-config.js configuration for contracts_directory: "contracts/*.sol" which solidity-coverage translates wrongly into: ls: no such file or directory: /argent-contracts/contracts/*.sol/**/*.sol

[cgewecke]

@elenadimitrova

I also have a problem with the root truffle-config.js configuration for contracts_directory: "contracts/.sol" which solidity-coverage translates wrongly into: ls: no such file or directory: /argent-contracts/contracts/.sol/*/.sol

That's definitely a bug.

For the other cases, do things work if you pass the config to coverage the way you would to compile?

npx truffle run coverage --config truffle-config-wallet.js

[elenadimitrova]

If we pass a single config as a time to coverage it fails to find artifacts from other compilations it depends on. For example running infrastructure coverage errors with a missing lib contract.

npx truffle run coverage --config truffle-config-infrastructure.js

produces

Error: Could not find /lib/paraswap/IAugustusSwapper.sol from any sources; imported from /argent-contracts/.coverage_contracts/IDexRegistry.sol

[cgewecke]

Yes, I see, that makes sense. Will add an option that lets you configure the location of the temp contracts correctly.

(It might be a few days before I publish these fixes (maybe the weekend) because I also need to get #548 out in the next release.)

[cgewecke]

@elenadimitrova Apologies for the delay here.

Believe 0.7.12 fixes the problem where contract_directory is a subfolder of contracts. There's no special option, it just locates the temporary instrumented contracts correctly now.

Unfortunately, the issue with glob magic in the contracts_directory path is not fixed. Solidity-coverage relies on that value being a folder in tons of places and handling globs for .sols as well would be very tricky. Do you have any possible work-around on your side for this?

[elenadimitrova]

Thanks @cgewecke . Trying 0.7.12 I am still not seeing compilation recognising the different truffle configs e.g.:

> solidity-coverage cleaning up, shutting down ganache server
CompileError: /argent-contracts/.coverage_contracts/infrastructure_0.5/CompoundRegistry.sol:16:1: ParserError: Source file requires different compiler version (current compiler is 0.6.12+commit.27d51765.Linux.g++) - note that nightly builds are considered to be strictly less than the released version
pragma solidity ^0.5.4;

This is if I purely run truffle run coverage which picks up the root config and ignores the rest.If I selectively run each config file via e.g. npx truffle run coverage --config truffle-config-infrastructure.js I get a different error which is missing files from other compilations:

> solidity-coverage cleaning up, shutting down ganache server
/argent-contracts/node_modules/truffle/build/library.bundled.js:350134
        throw new Error("Could not find artifacts for " + import_path + " from any sources");
        ^

Error: Could not find artifacts for Migrations from any sources

To be clear on the requirements here, compared to truffle tests where I need to run truffle compile nine times with nine different configs, I should be able to pass this same compilation setup to solidity-coverage.

[cgewecke]

@elenadimitrova

Is it that you usually run multiple compilations before the test run and the artifacts accumulate in the build folder?

truffle compile --config truffle-config-a.js
truffle compile --config truffle-config-b.js
...
truffle test --config truffle-config-test.js

If so, you can emulate this by adding the --temp build flag to the coverage command.

truffle compile --config truffle-config-a.js
truffle compile --config truffle-config-b.js
...

# This will deposit coverage artifacts in the `build` folder instead of a temporary staging folder.
# Coverage will automatically delete the `build` folder when it completes
truffle run coverage --temp build --config truffle-config-test.js

[EDIT] - if this doesn't work, will look directly at argent tomorrow.

[elenadimitrova]

Is it that you usually run multiple compilations before the test run and the artifacts accumulate in the build folder?

Yes. We run 9 compile commands which accumulate artifacts in the build as well as build-legacy folders. Commands are

npx truffle compile --config truffle-config-lib.js
npx truffle compile
npx truffle compile --config truffle-config-infrastructure-0.5.js
npx truffle compile --config truffle-config-infrastructure.js
npx truffle compile --config truffle-config-modules.js
npx truffle compile --config truffle-config-wallet.js
npx truffle compile --config truffle-config-contracts-legacy-1.3.js
npx truffle compile --config truffle-config-contracts-legacy-1.6.js
npx truffle compile --config truffle-config-contracts-test.js

If so, you can emulate this by adding the --temp build flag to the coverage command.

Trying this with npx truffle run coverage --temp build --config truffle-config-infrastructure.js removes the build folder containing all compiled artifacts and throws error :

> Istanbul reports written to ./coverage/ and ./coverage.json
> solidity-coverage cleaning up, shutting down ganache server
/argent-contracts/node_modules/truffle/build/library.bundled.js:350134
        throw new Error("Could not find artifacts for " + import_path + " from any sources");
        ^

Error: Could not find artifacts for Migrations from any sources
    at Resolver.require (/argent-contracts/node_modules/truffle/build/library.bundled.js:350134:15)

If you do look at the argent repo, please use the experiment/migrate-to-truffle branch as we have not yet migrated to truffle.

[cgewecke]

Ah ok! Sorry, I've been very slow to understand what's happening here 🙂 .

I think your coverage will need to be generated using a custom script that looks similar to the plugin code here but runs a loop which processes each config for instrumentation and compilation.

Other projects do this as well - for example dydx generates their coverage with a script because they use jest instead of mocha. You would still have solidity-coverage as a dep but consume it as an api rather than through the generic truffle plugin.

Will take a look at argent today and see if this can be done +/- simply.

[elenadimitrova]

Thanks for the PR @cgewecke this sorted the compilation pipeline issue so closing this. ps: you should add a "donation" button on the repo for those of us that want to give back. Thanks for continuing to maintain this. ❤️

[elenadimitrova]

@cgewecke any clues as to why coverage is reported differently for MakerV2Base, MakerV2Invest, MakerV2Loan and BaseWallet contracts? When running with truffle it's 0 and with etherlime it's 100%? These are all contracts called indirectly (i.e. The maker contracts are inherited by MakerV2Manager with is reported at 100% and BaseWallet uses a proxy underneath) so perhaps coverage doesn't register?

https://app.circleci.com/pipelines/github/argentlabs/argent-contracts/1468/workflows/c5168f25-0ac9-4792-b36f-3b8fbcf2a607/jobs/1792

modules/maker/                |        0 |        0 |        0 |        0 |                |
  IUniswapExchange.sol         |      100 |      100 |      100 |      100 |                |
  IUniswapFactory.sol          |      100 |      100 |      100 |      100 |                |
  MakerV2Base.sol              |        0 |      100 |        0 |        0 | 54,55,56,57,64 |
  MakerV2Invest.sol            |        0 |        0 |        0 |        0 |... 159,170,171 |
  MakerV2Loan.sol              |        0 |        0 |        0 |        0 |... 607,608,609 |
  MakerV2Manager.sol           |      100 |      100 |        0 |      100 |                |
 wallet/                       |     2.86 |        0 |    22.22 |     7.32 |                |
  BaseWallet.sol               |        0 |        0 |        0 |        0 |... 137,139,142 |
  IWallet.sol                  |      100 |      100 |      100 |      100 |                |
  Proxy.sol                    |      100 |      100 |      100 |      100 |                |

vs a run with etherlime here https://app.circleci.com/pipelines/github/argentlabs/argent-contracts/1462/workflows/f234068b-da3f-40c8-9fb5-b6998fc49880/jobs/1784/steps

modules/maker/                |      100 |       85 |      100 |      100 |                |
  IUniswapExchange.sol         |      100 |      100 |      100 |      100 |                |
  IUniswapFactory.sol          |      100 |      100 |      100 |      100 |                |
  MakerV2Base.sol              |      100 |      100 |      100 |      100 |                |
  MakerV2Invest.sol            |      100 |      100 |      100 |      100 |                |
  MakerV2Loan.sol              |      100 |    84.21 |      100 |      100 |                |
  MakerV2Manager.sol           |      100 |      100 |      100 |      100 |                |
 wallet/                       |      100 |    92.31 |      100 |      100 |                |
  BaseWallet.sol               |      100 |    92.31 |      100 |      100 |                |
  IWallet.sol                  |      100 |      100 |      100 |      100 |                |
  Proxy.sol                    |      100 |      100 |      100 |      100 |                |

[cgewecke]

Oh! Ok, will take a look.