From a0a4254f47743d55f4c2a4be167ec740b497bc04 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Mon, 2 Oct 2017 11:50:09 -0700 Subject: [PATCH 1/6] Revert vm stipend fix --- README.md | 1 + docs/faq.md | 6 ++++++ package.json | 2 +- test/app.js | 2 +- 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 05e5293..1350bf2 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,7 @@ Solutions to common issues people run into using this tool: + [Using alongside HDWalletProvider](https://github.com/sc-forks/solidity-coverage/blob/master/docs/faq.md#using-alongside-hdwalletprovider) + [Integrating into CI](https://github.com/sc-forks/solidity-coverage/blob/master/docs/faq.md#continuous-integration-installing-metacoin-on-travisci-with-coveralls) + [Why are asserts and requires highlighted as branch points?](https://github.com/sc-forks/solidity-coverage/blob/master/docs/faq.md#why-has-my-branch-coverage-decreased-why-is-assert-being-shown-as-a-branch-point) ++ [Why are `send` and `transfer` throwing in my tests?](https://github.com/sc-forks/solidity-coverage/blob/master/docs/faq.md#why-are-send-and-transfer-throwing) + [Running testrpc-sc on its own](https://github.com/sc-forks/solidity-coverage/blob/master/docs/faq.md#running-testrpc-sc-on-its-own) ### Example reports diff --git a/docs/faq.md b/docs/faq.md index 6915ede..5aa8f06 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -170,6 +170,12 @@ Clearly, the coverage should be the same in these situations, as the code is (fu If an `assert` or `require` is marked with an `I` in the coverage report, then during your tests the conditional is never true. If it is marked with an `E`, then it is never false. +### Why are send and transfer throwing? + +If you include contracts that have fallback function in the list of files to instrument and attempt to `send` or `transfer` to them, +the methods will throw because the instrumentation consumes more gas than these methods allow. See the `skipFiles` option in the +README to exclude these files and [issue 118](https://github.com/sc-forks/solidity-coverage/issues/118) for a more detailed discussion of +this problem. ### Running testrpc-sc on its own diff --git a/package.json b/package.json index 8326dc2..90465a3 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "dependencies": { "commander": "^2.9.0", "death": "^1.1.0", - "ethereumjs-testrpc-sc": "4.0.3", + "ethereumjs-testrpc-sc": "4.0.2", "istanbul": "^0.4.5", "keccakjs": "^0.2.1", "mkdirp": "^0.5.1", diff --git a/test/app.js b/test/app.js index fc0261c..c9b8b68 100644 --- a/test/app.js +++ b/test/app.js @@ -281,7 +281,7 @@ describe('app', () => { collectGarbage(); }); - it('contract sends / transfers to instrumented fallback: coverage, cleanup & exit(0)', () => { + it.skip('contract sends / transfers to instrumented fallback: coverage, cleanup & exit(0)', () => { // Validate ethereumjs-vm hack to remove gas constraints on transfer() and send() assert(pathExists('./coverage') === false, 'should start without: coverage'); assert(pathExists('./coverage.json') === false, 'should start without: coverage.json'); From 7973471e4e7eb20636cc418e3f1190b2862b3514 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Mon, 2 Oct 2017 12:13:51 -0700 Subject: [PATCH 2/6] 0.2.5 --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index d46b830..18031a3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "solidity-coverage", - "version": "0.2.4", + "version": "0.2.5", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 90465a3..30f46cc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "solidity-coverage", - "version": "0.2.4", + "version": "0.2.5", "description": "", "bin": { "solidity-coverage": "./bin/exec.js" From dcee2f61e35b42acde9b52d152c2f591c899cf34 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Mon, 2 Oct 2017 12:16:26 -0700 Subject: [PATCH 3/6] Update changelog: 0.2.5 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d04bce..4409c37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,10 @@ # Changelog + +0.2.5 / 2017-10-02 +================= + * Revert vm stipend fix, it was corrupting balances. `send` & `transfer` to instrumented fallback + will fail now though. + 0.2.4 / 2017-09-22 ================= From f9ddd876e91da271ef32dcb37627430a19640b2b Mon Sep 17 00:00:00 2001 From: Marcin Rudolf Date: Fri, 29 Sep 2017 16:54:36 +0200 Subject: [PATCH 4/6] reads trace file line by line --- lib/app.js | 59 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/lib/app.js b/lib/app.js index c467553..8ffdec1 100644 --- a/lib/app.js +++ b/lib/app.js @@ -2,6 +2,7 @@ const shell = require('shelljs'); const fs = require('fs'); const path = require('path'); const childprocess = require('child_process'); +const readline = require('readline'); const reqCwd = require('req-cwd'); const istanbul = require('istanbul'); const getInstrumentedVersion = require('./instrumentSolidity.js'); @@ -184,14 +185,6 @@ class App { shell.exec(command); this.testsErrored = shell.error(); shell.cd('./..'); - } catch (err) { - this.cleanUp(err); - } - - // Get events fired during instrumented contracts execution. - try { - this.events = fs.readFileSync('./allFiredEvents').toString().split('\n'); - this.events.pop(); } catch (err) { const msg = ` @@ -212,25 +205,39 @@ class App { const reporter = new istanbul.Reporter(); return new Promise((resolve, reject) => { - try { - this.coverage.generate(this.events, `${this.workingDir}/contracts`); - - const json = JSON.stringify(this.coverage.coverage); - fs.writeFileSync('./coverage.json', json); - - collector.add(this.coverage.coverage); - reporter.add('html'); - reporter.add('lcov'); - reporter.add('text'); - reporter.write(collector, true, () => { - this.log('Istanbul coverage reports generated'); - this.cleanUp(); - resolve(); + // Get events fired during instrumented contracts execution. + const stream = fs.createReadStream(`./allFiredEvents`); + stream.on('error', err => this.cleanUp('Event trace could not be read.\n' + err)); + const reader = readline.createInterface({ + input: stream, + }); + this.events = []; + reader + .on('line', line => this.events.push(line)) + .on('close', () => { + // Generate Istanbul report + try { + // console.log(`got ${this.events.length} events`); + this.coverage.generate(this.events, `${this.workingDir}/contracts`); + + const json = JSON.stringify(this.coverage.coverage); + fs.writeFileSync('./coverage.json', json); + + collector.add(this.coverage.coverage); + reporter.add('html'); + reporter.add('lcov'); + reporter.add('text'); + reporter.write(collector, true, () => { + this.log('Istanbul coverage reports generated'); + this.cleanUp(); + resolve(); + }); + } catch (err) { + const msg = 'There was a problem generating the coverage map / running Istanbul.\n'; + console.log(err.stack); + this.cleanUp(msg + err); + } }); - } catch (err) { - const msg = 'There was a problem generating the coverage map / running Istanbul.\n'; - this.cleanUp(msg + err); - } }); } From 31167a0cb3c76ec5587f5ca38452befd9b8c6708 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Mon, 2 Oct 2017 18:02:47 -0700 Subject: [PATCH 5/6] Fix tests that fail with stream read --- lib/app.js | 1 - test/app.js | 2 +- test/cli/command-options.js | 8 +++++--- test/util/mockTestCommand.js | 3 ++- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/app.js b/lib/app.js index 8ffdec1..c676c3e 100644 --- a/lib/app.js +++ b/lib/app.js @@ -217,7 +217,6 @@ class App { .on('close', () => { // Generate Istanbul report try { - // console.log(`got ${this.events.length} events`); this.coverage.generate(this.events, `${this.workingDir}/contracts`); const json = JSON.stringify(this.coverage.coverage); diff --git a/test/app.js b/test/app.js index c9b8b68..51853da 100644 --- a/test/app.js +++ b/test/app.js @@ -78,7 +78,7 @@ describe('app', () => { assert(pathExists('./allFiredEvents') === false, 'should start without: events log'); const testConfig = Object.assign({}, config); - testConfig.testCommand = 'mocha --timeout 5000 > /dev/null 2>&1'; + testConfig.testCommand = 'mocha --timeout 5000'; testConfig.dir = './mock'; testConfig.norpc = false; testConfig.port = 8888; diff --git a/test/cli/command-options.js b/test/cli/command-options.js index d1a5763..8dad758 100644 --- a/test/cli/command-options.js +++ b/test/cli/command-options.js @@ -4,18 +4,20 @@ const assert = require('assert'); const fs = require('fs'); // Fake event for Simple.sol -const fakeEvent = { +const fakeEvent = {"address":"6d6cf716c2a7672047e15a255d4c9624db60f215","topics":["34b35f4b1a8c3eb2caa69f05fb5aadc827cedd2d8eb3bb3623b6c4bba3baec17"],"data":"00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000003a2f55736572732f757365722f53697465732f73632d666f726b732f6d657461636f696e2f636f6e7472616374732f4d657461436f696e2e736f6c000000000000"} + +/* { address: '7c548f8a5ba3a37774440587743bb50f58c7e91c', topics: ['1accf53d733f86cbefdf38d52682bc905cf6715eb3d860be0b5b052e58b0741d'], data: '0', -}; +};*/ // Tests whether or not the testCommand option is invoked by exec.js // Mocha's default timeout is 2000 - here we fake the creation of // allFiredEvents at 4000. describe('Test uses mocha', () => { it('should run "mocha --timeout 5000" successfully', done => { setTimeout(() => { - fs.writeFileSync('./../allFiredEvents', fakeEvent); + fs.writeFileSync('./../allFiredEvents', JSON.stringify(fakeEvent) + '\n'); done(); }, 4000); }); diff --git a/test/util/mockTestCommand.js b/test/util/mockTestCommand.js index 3222606..d0abb34 100644 --- a/test/util/mockTestCommand.js +++ b/test/util/mockTestCommand.js @@ -2,6 +2,7 @@ const fs = require('fs'); const request = require('request'); +const fakeEvent = {"address":"6d6cf716c2a7672047e15a255d4c9624db60f215","topics":["34b35f4b1a8c3eb2caa69f05fb5aadc827cedd2d8eb3bb3623b6c4bba3baec17"],"data":"00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000003a2f55736572732f757365722f53697465732f73632d666f726b732f6d657461636f696e2f636f6e7472616374732f4d657461436f696e2e736f6c000000000000"} request({ uri: 'http://localhost:8888', @@ -17,6 +18,6 @@ request({ console.error(error); process.exit(1); } - fs.writeFileSync('../allFiredEvents', 'foobar'); + fs.writeFileSync('../allFiredEvents', JSON.stringify(fakeEvent) + '\n'); process.exit(0); }); From cedd3ee7c9afaf5c84f7329637634e79460d2b0d Mon Sep 17 00:00:00 2001 From: c-g-e-w-e-k-e- Date: Mon, 2 Oct 2017 18:43:28 -0700 Subject: [PATCH 6/6] Add rudolfix to contributors list. --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1350bf2..de89292 100644 --- a/README.md +++ b/README.md @@ -113,4 +113,5 @@ also lint your submission with `npm run lint`. Bugs can be reported in the + [@cgewecke](https://github.com/cgewecke) + [@adriamb](https://github.com/adriamb) + [@cag](https://github.com/cag) -+ [@maurelian](https://github.com/maurelian) \ No newline at end of file ++ [@maurelian](https://github.com/maurelian) ++ [@rudolfix](https://github.com/rudolfix)