sc-forks / solidity-coverage

Code coverage for Solidity smart-contracts
MIT License
970 stars 261 forks source link

Feature request. Relative path for node_modules and exclude filter #264

Closed sergeyfsv-da closed 5 years ago

sergeyfsv-da commented 6 years ago

I'd like to extract all smart contract related files into a separate directory. .\smart-contract\, for example. It content could be \contracts \migrations \tests .solcover.js However, right now I cannot do this, because my smart contract depends on openzeppelin-solidity, and it seems that copyPackages is looking for node_modules folder only in the current directory.

I also do not want to put my .solcover into the root directory of the repository, because it copies everything into the coverageEnv folder which is the time-consuming process. Moreover, it copies .git folder and there is no a mechanism that could prevent this.

cgewecke commented 6 years ago

@sergeyfsv-da To be clear you'd like to organize your project like this?

top/
  smart-contracts/
    contracts/
    migrations/
    etc..
  something-else/
     ..
  even-more/
  node_modules/

it seems that copyPackages is looking for node_modules folder only in the current directory.

FWIW #236 added support for symbolic links so that solidity-coverage could be used with Lerna/Yarn-workspaces. In that pattern there are node_modules local to each subdirectory but their contents are hoisted to the top level and yarn manages everything with symlinks.

Additionally, with the newer version of Truffle (4.1.13) you might not have to copy node_modules packages anymore. Truffle should resolve solidity imports in solidity files by looking recursively outwards from a folder. Could you try that version after unsetting copyNodeModules (or copyPackages) and see if it works?

cgewecke commented 6 years ago

@sergeyfsv-da Out of curiosity, why is copying the .git folder a problem? We could easily filter that out...

cgewecke commented 5 years ago

Duplicate of #297 (also closed)

PaulRBerg commented 5 years ago

In that pattern there are node_modules local to each subdirectory but their contents are hoisted to the top level and yarn manages everything with symlinks.

This is not the case with yarn workspaces, since modules are not hoisted to each subdirectory by default. They can be "dehoisted" only if you manually do so via nohoist.

My particular situation is a pain in the neck. I managed to sort out the @openzeppelin/contracts-ethereum-package module by doing this:

{
  ...
  "nohoist": [
    "**/@openzeppelin/contracts-ethereum-package",
    "**/@openzeppelin/contracts-ethereum-package/**"
  ]
  ...
}

But I also have another local module called @sablier/protocol that I'm importing in @sablier/payroll.

If I add it to the "nohoist" list:

{
  ...
  "nohoist": [
    "**/@openzeppelin/contracts-ethereum-package",
    "**/@openzeppelin/contracts-ethereum-package/**",
    "**/@sablier/protocol",
    "**/@sablier/protocol/**"
  ]
  ...
}

I end up getting this error:

error An unexpected error occurred: "ENOENT: no such file or directory, lstat '/Users/paulrberg/Projects/Sablier/Sablier/packages/dev-utils/node_modules/mocha'"

I understand that importing "node_modules" from multiple paths other than the relative one can be quite cumbersome for you, but it would be a really neat feature to have for 0.7.0.

Cc https://github.com/sc-forks/solidity-coverage/pull/372

cgewecke commented 5 years ago

@PaulRBerg This should be resolved by 0.7.0 because it stores instrumented solidity files in a temporary folder at the same layer as contracts. Right before coverage is run the project looks like this:

.coverage_artifacts /
.coverage_contracts /
contracts /
migrations /
node_modules /
test /
truffle-config.js

Any path which truffle resolves correctly for yarn workspaces should work. copyPackages and copyNodeModules are no longer necessary in 0.7.0.

@PaulRBerg For a project with sabliers structure it might be worth waiting a couple days for the first 0.7.0 beta publication because many of the contortions you'd currently need to do to get things working have been simplified away in that version.

PaulRBerg commented 5 years ago

Any path which truffle resolves correctly for yarn workspaces should work

This would be legend!

It might be worth waiting a couple days

Sounds good. We do have an audit commencing next week, but I think we could get away with copy-and-pasting the contracts for the time being.

cgewecke commented 5 years ago

@PaulRBerg Ah! I think I will be publishing tomorrow AM (GMT -8) if all goes well today so hopefully you'll be ok.

PaulRBerg commented 5 years ago

Amazing! Thank you @cgewecke, your support these days has been exceptionally helpful. I logged in my @SablierBot account to double-star this repo.