Add option to include other folders in the coverage envelope

[cgewecke]

melonproject/protocol for example has a utilities folder located at project root which the tests require from e.g. require('../utilities/<file>')

[area]

We had this issue internally as well. The solution I've been using is

files = shell.ls(`${workingDir}`)
shell.mkdir(`${coverageDir}`);
shell.cp('-R', files, `${coverageDir}`);

to generate the coverage environment. Adding individual folders, like ../utilities in the case of melonport is one thing, but we also included things from node_modules, so we need to at least copy what's needed from there as well, and at that point it seems like it'd be easier to just copy everything and not require users to configure it to get solidity-coverage to work. Perhaps a configurable override if they know they don't need to copy everything, and performance is an issue?

[cgewecke]

@area Yes I agree, this is a really good idea. If SC is going to use this 'coverageEnv' approach it should just copy the whole project into it . . . and maybe also ask people to declare a coverage network with the right gas values instead of trying to handle the default case for them.

The only reservation I have about this is copying node_modules by default because a) it's usually massive b) you should be able to require packages from anywhere and have them resolve correctly since node recursively searches outwards until it finds something.

Are you having the node_modules issue in migrations.js? How is it coming up? It seems like this problem might be about truffle overloading require incorrectly and is a bug they've scheduled a fix for by v3.3.0.

An interim solution might be to adopt your approach (it's simpler and cleaner anyway), but exclude node_modules by default with an option to include?

[area]

It's actually a mocha reporter that we tell it to use in truffle.js that ends up not being able to be found. It's possible I'm to blame, given I added the ability to editmocha options in truffle...

EDIT: This appears to be a consequence of using the global truffle (mocha only searches in ${cwd}/node_modules), which I guess raises the question: should we be installing our own truffle that we can run locally?

[cgewecke]

Hmmm! Interesting! @area Someone fixed the require bug and it was pushed to truffle master. Is there a chance you could check to see if the reporter works using the #master? If not . . . this is fairly serious problem.

Another remotely possible fix: people are exposing the babel transpiler globally to their truffle project by requiring it at the top of truffle.js and this resolves without issue inside the coverage env. Discussion here. I wonder if something similar could be done with mocha. . . (unlikely).

Maybe we should add truffle! - it's dev dependency right now (to speed up CI). We already add another whole testrpc. This thing will be so big though . . .

[cgewecke]

Published with 0.1.4

[fumecow]

The new truffle 3.3 seems to break requiring local .js files (not a package in node_modules). Truffle 3.2.5 allowed a workaround to require('../file.js') if the file was outside the node_modules directory but this does not work anymore in 3.3. require('file.js') does not work either.

Truffle 3.3 gives an error: Error: Cannot find module "."

[tcoulter]

@fumecow Truffle 3.3.1 has been released which should include a require fix. Can you give it a shot?

[fumecow]

yes - will do. Thanks!

[fumecow]

I'm not seeing any difference. But I do notice that when I pulled the latest truffle using npm, truffle --version still says 3.3.0 even though package.json says ^3.3.1. Did the update not make it into the package somehow?

[tcoulter]

Hmm, let me take a look.

[tcoulter]

Did your bundle version increase? i.e.,

$ truffle version
Truffle v3.3.0, bundle version: 3.3.1
Solidity v0.4.11 (solc-js)

[tcoulter]

If it did, can you give me some code that easily reproduces the issue? That will help tremendously.

[tcoulter]

We should probably change that (new) version number to be reversed: Bundle version is more important than core version.

[fumecow]

Yes - the bundle version did increment to 3.3.1. I'm seeing the error for example on the initial migration when I remove the ".." in the following require. It also happens with the tests. In this case, the config.js file is in the project's home directory, one up from the migrations directory.

const config = require('../config.js')

var Migrations = artifacts.require("Migrations.sol")

module.exports = function(deployer) {
  deployer.deploy(Migrations, { gas: config.gas.test })
};

Hope that helps reproduce..

[cgewecke]

@tcoulter

This looks like its webpack injecting garbage into the dynamic require statements in truffle-require. A possible fix is to add a noParse config option for that file, e.g

// cli.webpack.config.js
...
module: {
    rules: [
      { test: /\.js$/, use: "shebang-loader" }
    ],
    noParse: [/truffle-require/]
  },
...

Build is too big to link a line ref to on GitHub but tested this and it seems to preserve the requires appropriately at that spot in the bundle.

EDIT: This obviously isn't enough since there are requires at the top of that file that DO need webpack injections.

SECOND EDIT: Maybe it does work! If not it might be possible to extract the require function block into its own file and noParse it, since that code doesn't look like it has any non-node dependencies.

[tcoulter]

Thanks for the tip @cgewecke. Much appreciated.

I've just released version v3.3.2, and as far as I can tell it fixes the error. Please give it a shot. Thank you to all for your patience and feedback.

Cheers!

[fumecow]

Confirmed - this seems to fix it here as well. Thanks!

[cgewecke]

@tcoulter @fumecow Maybe I'm crazy It seems like it might still be broken - just slightly differently 😄 My test case involves requiring from a folder outside the tests folder and with 3.3.2 (installed globally or locally) I get this error:

Error: Cannot find module 'truffle-expect'
    at Function.Module._resolveFilename (module.js:469:15)
    at Function.Module._load (module.js:417:25)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:85773:14)
    at __webpack_require__ (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:21:30)
    at Object.<anonymous> (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:59914:15)
    at __webpack_require__ (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:21:30)
    at Object.<anonymous> (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:201851:15)
    at __webpack_require__ (/usr/local/lib/node_modules/truffle/build/cli.bundled.js:21:30)

That kind of makes sense because in 3.3.1 webpack was also injecting bundle references for the module requires at the top of truffle-require/require.js.

Don't we have to extract the function below into it's own file, noParse that, but require it into require.js?:

require: function(pkgPath) {
          // Ugh. Simulate a full require function for the file.
          pkgPath = pkgPath.trim();

          // If absolute, just require.
          if (path.isAbsolute(pkgPath)) {
            return require(pkgPath);
          }
         ... etc 

I'm going to post a link to a repo that can reproduce the error in a second.

[cgewecke]

@tcoulter: repo is metacoin-require-bug. It has 3.3.2 installed as a dev dependency so running ./node_modules/.bin/truffle test should reliably reproduce the error. Tests pass with 3.2.5 though.

Sorry about the original suggestion . . . I think that little block of code can be extracted into an unparsed file and still work - it only needs modules that are available from node so its probably okay if they're not bundled.

[fumecow]

I guess I forgot to mention I had to 'npm install' the new (missing) modules manually..

[tcoulter]

Truffle 3.4.3 is out. As far as I can tell, this should fix the issue for good. The reason this sprung back up was due to bundling issues; our tests were passing unbundled, but didn't catch require failures in the bundled version. We're working on reorganizing these tests.

If one of you can test out the new version to ensure I've fixed the issue for you, that'd be much appreciated.

[cgewecke]

@tcoulter YES!!! Works. Thank you!

[fumecow]

@tcoulter I can confirm this is working here as well. Thanks!