Solidity-coverage: Version 0.4.2 performance degraded

[elenadimitrova]

Upgraded from 0.4.0 to 0.4.2 and run time went up from 5 to 13 minutes for a total of 104 tests.

In addition I started getting warnings for contracts found in node_modules which was not the case before and no other changes besides solidity-coverage upgrade were made.

Warning: The file at ./coverageEnv/node_modules/solidity-parser-antlr/solidity-antlr4/test.sol was identified as a Solidity Contract, but did not parse correctly. You may ignore this warning if it is not a Solidity file, or your project does not use it
Warning: The file at ./coverageEnv/node_modules/solidity-parser-antlr/test/test.sol was identified as a Solidity Contract, but did not parse correctly. You may ignore this warning if it is not a Solidity file, or your project does not use it
Warning: The file at ./coverageEnv/node_modules/solium/test/lib/rules/double-quotes/accept/double-quoted.sol was identified as a Solidity Contract, but did not parse correctly. You may ignore this warning if it is not a Solidity file, or your project does not use it
Warning: The file at ./coverageEnv/node_modules/solium/test/lib/rules/double-quotes/reject/single-quoted.sol was identified as a Solidity Contract, but did not parse correctly. You may ignore this warning if it is not a Solidity file, or your project does not use it
Warning: The file at ./coverageEnv/node_modules/solium/test/lib/rules/quotes/double-quoted.sol was identified as a Solidity Contract, but did not parse correctly. You may ignore this warning if it is not a Solidity file, or your project does not use it
Warning: The file at ./coverageEnv/node_modules/solium/test/lib/rules/quotes/single-quoted.sol was identified as a Solidity Contract, but did not parse correctly. You may ignore this warning if it is not a Solidity file, or your project does not use it
Warning: The file at ./coverageEnv/node_modules/solparse/test/doc_examples.sol was identified as a Solidity Contract, but did not parse correctly. You may ignore this warning if it is not a Solidity file, or your project does not use it

[cgewecke]

Thanks @elenadimitrova.

The warnings were introduced with this PR to fix a bug caused by our own test suite. Additionally we are now sweeping across a staggering number of files to address problems raised by thepure and view restrictions.

In any case, hoping to publish 0.4.3 by tomorrow which will include a recent PR that lets you specify individual packages from node_modules to include rather than the entire folder. This will require changing your .solcover.js to use the new option but should be quite a bit faster.

[cgewecke]

0.4.3? The new option is called: copyPackages and I believe it's just:

copyPackages:  ['zeppelin-solidity']

Also think it's no longer necessary to use copyNodeModules option for JS packages required by your tests, like chai or whatever. It should only be required when a solidity file imports from a node_modules package.

Leaving this open, pending your assessment. Also, we're now double-compiling the contracts, so it will still be a little slower.

[elenadimitrova]

I had trouble getting the copyPackages option to work so @area helped me. This is not documented in the readme so posting my (simplified) working SC config here:

module.exports = {
    copyPackages: [],
    skipFiles: [
      'Migrations.sol',
    ],
    compileCommand: '../node_modules/.bin/truffle compile',
    testCommand: '../node_modules/.bin/truffle test --network coverage',
    testrpcOptions: `--port 8555 -i coverage`
};

Notable changes were

• adding the empty copyPackages: [] and removing copyNodeModules
• adding a reference to the upper level node_modules i.e. ../node_modules
• adding a compileCommand

Run time dropped down to acceptable level for us (7 min) so happy to close this.

[cgewecke]

Oh hmm. Yes. That makes sense. I think we should definitely add something to the README about locally scoped truffle / testrpc. Thanks again for debugging here.

[cgewecke]

@elenadimitrova Apologies, there's something I don't understand from your example above. Do you know if your config will work without the empty copyPackages array? I feel like it should, and if it doesn't we need to fix that.

[elenadimitrova]

Confirming it works without copyPackages option declared @cgewecke

[cgewecke]

Leaving this issue open and marking as a bug since although there's a workaround, it's not really fixed / will probably be a problem for others.

[cgewecke]

Hi @elenadimitrova. Hope you have a wonderful new year, you're the best.

This should be fixed in 0.7.0. No longer copying any packages anywhere or changing people's modifiers.