Support for npm scripts which call npm packages

[maurelian]

Try to run this on /0xProject/contracts. The tests run as is without issue (using npm run test, but throw an error when called via solidty-coverage. It seems to have something to do with the npm scripts:

"transpile": "rm -rf ./transpiled; copyfiles ./build/**/* ./transpiled; tsc",
"test": "npm run transpile; truffle test",

Error message:

Launching test command (this can take a few seconds)...

> 0x-smart-contracts@1.0.0 test /Users/primary/Projects/0x_contracts/coverageEnv
> npm run transpile; truffle test


> 0x-smart-contracts@1.0.0 transpile /Users/primary/Projects/0x_contracts/coverageEnv
> rm -rf ./transpiled; copyfiles ./build/**/* ./transpiled; tsc

sh: copyfiles: command not found
sh: tsc: command not found

npm ERR! Darwin 16.6.0
npm ERR! argv "/Users/primary/.nvm/versions/node/v7.10.0/bin/node" "/Users/primary/.nvm/versions/node/v7.10.0/bin/npm" "run" "transpile"
npm ERR! node v7.10.0
npm ERR! npm  v4.2.0

I also tried changing the script to

rm -rf ./transpiled; ./node_modules/.bin/copyfiles ./build/**/* ./transpiled; ./node_modules/.bin/tsc

This gets me a new error, which seems closer to success:

error TS5023: Unknown compiler option 'baseUrl'.
error TS5023: Unknown compiler option 'allowJs'.

[cgewecke]

So . . . in principal we do support this: since #44 it's possible to set a copyNodeModules option in .solcover.js to true and include node_modules in the coverage environment. For 0xProject this adds about 90 seconds to the total time coverage takes to run. And it's possible to get a report this way (recipe below).

However, I'm seeing some bewildering results:

• running tests w/out coverage: 58 passing / 18 failing.
• running test with coverage: 25 passing / 51 failing.

I don't understand why. Could be something in the transpilation step? Or not? Complicated build here! They're pushing the envelope.

@maurelian Are you getting 58 passing / 18 failing running the tests as they suggest? If everything's passing for you then something is wrong on my end.

As far as configuration goes:
My .solcover.js looks like this:

module.exports = {
    testCommand: 'npm run transpile; truffle test --network coverage',
    copyNodeModules: true,
}

My truffle.js looks like this:

module.exports = {
  networks: {
    development: {
      host: 'localhost',
      port: 8545,
      network_id: '*', // Match any network id
    },
    kovan: {
      host: 'localhost',
      port: 8546,
      network_id: '42',
      gas: 4612388,
    },
    coverage: {
      host: "localhost",
      network_id: "*",
      port: 8555,         // <-- Use port 8555  
      gas: 0xfffffffffff, // <-- Use this high gas value
      gasPrice: 0x01      // <-- Use this low gas price
    }
  },
  test_directory: 'transpiled/test',
  migrations_directory: 'transpiled/migrations',
};

NB: also fooled around with the network id param (b/c they specify one) in both testrpc and truffle.js and it made no noticeable difference.

[maurelian]

I had the tests running at some point, on some branch, but similarly am getting failures now.

Sorry for sending you on a bug hunt that might not have been in your tool. Let me get back to you on this soon after talking to the team. :)

[cgewecke]

Oh not at all - I think there are definitely solidity-coverage bugs here! No worries.

[cgewecke]

I'm going to close because the question about how to call node packages from scripts is basically resolved. However - I'm adding 0xProject to the list of installation targets to test. And think an addendum to the README might be in order here because how to handle this issue is not super clear.

[maurelian]

I know this is closed, but I've made some progress, and also hitting a new wall! So it's worth reporting in. :)

Updates to the repo are here: https://github.com/maurelian/0x-contracts/tree/audit_coverage

Things I worked around:

• 0x' tests are very sensitive to the testrpc version, so it needs to be @3.0.2 to get the tests running with their npm scripts.
• I had to make this change to appease the parser. I don't think it would impact the report output
• I got tsc working by modifying the options.

So at this point I'm getting a funny error: /bin/sh: ./node_modules/truffle/cli.js: No such file or directory

but it does exist.

$ ./node_modules/truffle/cli.js version
Truffle v3.2.5

[maurelian]

I fixed the previous error by using the absolute path to truffle.

The report is at least generating now, but I'm running out of gas, even after adding ffff to the gas limit.

[cgewecke]

Hmmmm. Interesting. This is great.

Will you try adding --network coverage to the invocation of truffle in the testCommand option and seeing if that changes things? Alternatively you could make the development network have the same config as the coverage network.

I might also revert the change to the gas in truffle.js unless it still runs out of gas when deploying the contract. I remember I used to make the gas values really huge and some are too big - they get ignored or fail silently.

[maurelian]

So, when I tried that, I was getting empty responses from testrpc.

Error: Invalid JSON RPC response: ""

Then I tried norpc:true, and just ran your fork in another terminal:

./node_modules/ethereumjs-testrpc-sc/bin/testrpc --gasLimit 0xfffffffffff --networkId 50 --port 8555

And that worked! No idea what would be different, but it worked. That seems really weird to me, not sure what's up. Maybe some async stuff?

I also should double check the report output against the actual tests, for accuracy.

[cgewecke]

Oh great! The testrpc disconnect thing is really frustrating. I can't figure that out. I'm going to add your norpc solution should to the README under Known Issues because it's happening a fair amount to other people too.

[maurelian]

although I think running testrpc in-memory is more elegant, I generally appreciate the feedback of running it in a separate terminal session.

[cgewecke]

Yes, I agree. I hoped that in-memory would just work and would make integration with CI more straightforward but it's actually been an unrelenting source of confusion and unreliability.

[maurelian]

Reviewing the report, I'm pretty sure it's missing somethings.

ie. you would at least expect to see the the if statements in this function being run, right?

(as seen here)

Happy to file an issue if you have a suggestion for what to call it. :)

[cgewecke]

Yes definitely file an issue as long the tests are actually making it past the require at line 126. . . I don't know what to call that either. Maybe 'Missing coverage after require' and link here.

[cgewecke]

They have some serious tests for that so if it's not making it past the require they have bug, which is conceivable. . .it might be worth logging what the values are around here. When I search the report for fillOrder the coverage looks consistent with not running that code. Like you can see that fillOrders don't complete.

Which doesn't mean there isn't a problem here, but if that's the case shouldn't their tests error?

[cgewecke]

Checked the difference between testrpc 3.0.2 and testrpc-sc (which is based on 3.0.3). and saw that solc was temporarily pinned to 4.0.6 in that patch. I'm going to float it with a caret again . . . don't know if that will change things but maybe.

[cgewecke]

Actually I'm pinning it to 0.4.8 because that's what testrpc@latest is.
Edit - I can't check this until tomorrow but I wonder if solc floating in 3.0.2 results in the latest solc getting installed for them . . . require and assert are part of 0.4.10.

[maurelian]

Is the solc version used determined by the testrpc version?

I'm not getting any compilation errors, and the fillOrder errors are all complaining about an invalid opcode, which is consistent with the evaluation of require(false).

I'm experiencing a discrepancy between results which seem to vary depending on both the use of testrpc and when using solidity-coverage to run tests.

Quick summary of my observations:

1. With the vanilla ethereumjs-testrpc@3.02 and npm run test command: All tests are passing.

2. With ./node_modules/ethereumjs-testrpc-sc/bin/testrpc (no options), and npm run test command: fillOrder tests fail with the invalid opcode exception. I get 58 passing (26s) 38 failing
   (npm-test-output.txt)

3. With ./node_modules/ethereumjs-testrpc-sc/bin/testrpc --gasLimit 0xfffffffffff --port 8555, and ./node_modules/solidity-coverage/bin/exec.js I get the same invalid opcode errors on fillOrder tests, but 66 passing (19s) 30 failing (solidity-coverage-output.txt)

Any suggestions for troubleshooting? Feel free to ping me in slack. :)

[cgewecke]

I think the solc thing is irrelevant actually. I saw it in the testrpc deps but it looks like it's only used for the unit tests. I'm going to try to reproduce . . .

Were I to make another wild guess, this problem is about signing and weird signing stuff somewhere from earlier this year.

[maurelian]

I tried a variation on case 2 above, to see if your testrpc fork depends on the gas limit setting.

• With ./node_modules/ethereumjs-testrpc-sc/bin/testrpc --gasLimit 0xfffffffffff, and npm run test I get 66 passing (20s) 30 failing.

This at least brings the behavior of ./node_modules/solidity-coverage/bin/exec.js and npm run test in line with each other, and seems to suggest that the issue is with ethereumjs-testrpc-sc. mishandling something?

Should I move this to an issue on that repo?

[cgewecke]

Oh here is fine. This is sc-core. Could you npm list ethereumjs-util on your terminal and post the output so I can compare with what I have locally?

[maurelian]

$ npm list ethereumjs-util
0x-smart-contracts@1.0.0 /Users/primary/Projects/0x_contracts
├─┬ ethereumjs-abi@0.6.4
│ └── ethereumjs-util@4.5.0 
├── ethereumjs-util@5.1.2 
└─┬ solidity-coverage@0.1.7
  └─┬ ethereumjs-testrpc-sc@3.0.3 (git+https://github.com/sc-forks/testrpc-sc.git#15cc0fb8e031bee152c7b8e3f8df5f2ad33ca04f)
    ├─┬ ethereumjs-account@2.0.4
    │ └── ethereumjs-util@4.5.0 
    ├─┬ ethereumjs-block@1.2.2
    │ └── ethereumjs-util@4.5.0 
    ├─┬ ethereumjs-tx@1.1.2
    │ └── ethereumjs-util@4.5.0 
    ├─┬ ethereumjs-vm@2.0.2 (git+https://github.com/sc-forks/ethereumjs-vm-sc.git#328771cace30ed16ad5c1d13a69cda13d31173f9)
    │ └── ethereumjs-util@4.5.0 
    ├─┬ ethereumjs-wallet@0.6.0
    │ └── ethereumjs-util@4.5.0 
    └─┬ merkle-patricia-tree@2.1.2
      └── ethereumjs-util@4.5.0 

[cgewecke]

Oh and where is your testrpc3.0.2? Is that using ethereumjs-util@4.5.0?

[maurelian]

It's in ~/.config/yarn/global/node_modules/ethereumjs-testrpc

When I cd there:

$ npm list ethereumjs-util
ethereumjs-testrpc@3.0.2 /Users/primary/.config/yarn/global/node_modules/ethereumjs-testrpc
├─┬ ethereumjs-block@1.2.2
│ ├─┬ ethereumjs-tx@1.3.0
│ │ └── ethereumjs-util@5.1.1 
│ └── UNMET DEPENDENCY ethereumjs-util@^4.0.1
├─┬ ethereumjs-tx@1.1.2
│ └── UNMET DEPENDENCY ethereumjs-util@^4.5.0
└── UNMET DEPENDENCY ethereumjs-util@~4.5.0
npm ERR! missing: ethereumjs-util@~4.5.0, required by ethereumjs-testrpc@3.0.2
npm ERR! missing: ethereumjs-util@^4.0.1, required by ethereumjs-block@1.2.2
npm ERR! missing: ethereumjs-util@^4.5.0, required by ethereumjs-tx@1.1.2

[cgewecke]

Hmmmm! Interesting! Well, I made a branch of solidity-coverage with a testrpc-sc that uses ethereumjs-util@4.5.0:

npm install --save-dev https://github.com/sc-forks/solidity-coverage.git#district

For the life of me cannot get past the typescript comp errors and it seems like the require paths in the tests on the audit_coverage branch are unexpected . . . I'm getting this right after the compilation step:

Error: Cannot find module '../../build/contracts/MultiSigWalletWithTimeLock.json'

Thanks for that slack invite - I read through a few weeks of it on Monday and it's completely fascinating! So cool.

[maurelian]

Error: Cannot find module '../../build/contracts/MultiSigWalletWithTimeLock.json'

IIRC that's fixed by running npm run compile before npm run test.

[cgewecke]

Ok. Revisiting, now . . .

[cgewecke]

Sadly I'm not making too much headway on this. . . Some of the errors are being triggered by network stuff that's hardcoded into the migrations files - there are places there where the testing environment network is assumed to be 'development'. That stuff is fixable / hackable.

But the signing? Is there any way that you can ask them about their signing protocol and if they are dependent on ethereumjs-util@4.x.x? Just plugging in an earlier version of the utils isn't working.

There was significant change to the way things are signed this spring - see this ethereumjs-util PR and I think testrpc-sc is current in that regard, but I need to verify that and will write a test for it tomorrow. It's really important.