Merge pull request #86 from Neufund/rfix/removes-revert-from-tests

removes revert from tests

.solcover.js

@@ -1,4 +1,7 @@
 module.exports = {
+  port: 8255,
+  testCommand: 'truffle test --network coverage',
+  skipFiles: ['SnapshotToken/Extensions/Disbursal.sol', 'SnapshotToken/Extensions/Vote.sol'],
   testrpcOptions:
     '--port 8555 --gasLimit 0xfffffffffff --gasPrice 1 \
   --account="0x47be2b1589bb515b76b47c514be96b23cd60ee37e81d63c2ae9c92f7d7667e1a,12300000000000000000000000" \

README.md

@@ -21,6 +21,12 @@ truffle compile --all
 ```
 Truffle is not able to track dependencies correctly and will not recompile files that import other files
 
+You should consider replacing javascript compiler with `solc`, this will increase your turnover several times. Use following patches over cli.bundle.js (into which truffle is packed)
+```
+--- var result = solc.compileStandard(JSON.stringify(solcStandardInput));
++++ var result = require('child_process').execSync('solc --standard-json', {input: JSON.stringify(solcStandardInput)});
+```
+
 ### Auto fixing linting problems
 ```
 yarn lint:fix
@@ -32,14 +38,59 @@ yarn test:coverage
 ```
 
 you will find coverage report in `coverage/index.html`.
+We are using version custom version of `solidity-coverage`. Versions later than `0.2.2` introduce a problem as described in
+https://github.com/sc-forks/solidity-coverage/issues/118
+which results in balances increasing due to code execution and basically the result balance is unpredictable due to returned stipend.
+This issue prevents test that check balances to run properly.
+
+Custom version fixes two other bugs:
+1. For large trace files, `readFileSync` will fail silently, stream is used to read lines instead
+2. `exec` on child process will kill child if stdout buffer overflows, buffer was increased to 10MB
+3. It refers to testrpc `4.0.1` that has stipend not modified.
+
+Solidity code coverage runs own testrpc node (modified). You can run this node via
+```
+./node_modules/ethereumjs-testrpc-sc/build/cli.node.js --gasPrice 1 --gasLimit 0xfffffffffff -v
+```
+and execute tests via `coverage` network to check coverage behavior.
 
 ### Testing
+To run all tests, use the following
+```
+yarn test
+```
+
 To run single test, use following syntax
 ```
-truffle test test/LockedAccount.js test/setup.js
+yarn truffle test test/LockedAccount.js test/setup.js
 ```
 
 To run single test case from a test use following syntax
 ```
 it.only('test case', ...
 ```
+
+*Remarks on current state of tests in truffle and testrpc*
+
+Applies to `truffle 3.4.9` with `testrpc 4.0.1`.
+
+Truffle uses snapshotting mechanism (`evm_snapshot` and `evm_revert`) to revert to clean state between test suites. Current version of testrpc does not handle it correctly and will fail
+on revert with some probability. This makes running large test suites hard as there is high chance of testrpc to crash.
+
+As snapshotting is used to recover blockchain state after deployment scripts, we have no use of that mechanism and it can be disabled. Here is a patch to test runner
+https://github.com/trufflesuite/truffle-core/blob/master/lib/testing/testrunner.js
+```
+TestRunner.prototype.resetState = function(callback) {
+  callback();
+ };
+```
+
+Snapshotting has other problems that also makes it useless for state management in our tests.
+https://github.com/trufflesuite/ganache-core/issues/7
+Hopefully PRs solving this are pending.
+
+### Neufund modified Truffle
+
+Modified version of truffle is referenced for running test cases.
+1. Revert and snapshot are removed from `truffle-core` (https://github.com/Neufund/truffle-core/commit/83404a758a684e8d3d4806f24bc40a25c0817b79)
+2. https://github.com/trufflesuite/truffle/issues/569 is fixed as testing overloaded `transfer` is impossible (https://github.com/Neufund/truffle-contract/commit/ecae09942db60039f2dc4768ceeb88776226f0ca)

migrations/1_initial_migration.js

@@ -1,5 +1,7 @@
 const Migrations = artifacts.require("./Migrations.sol");
 
-module.exports = function deployMigration(deployer) {
+module.exports = function deployMigration(deployer, network) {
+  // do not deploy testing network
+  if (network === "inprocess_test" || network === "coverage") return;
   deployer.deploy(Migrations);
 };

migrations/2_deploy_contracts.js

@@ -23,6 +23,9 @@ const MIN_TICKET_EUR = web3.toBigNumber("300").mul(Q18);
 const ETH_EUR_FRACTION = web3.toBigNumber("300").mul(Q18);
 
 module.exports = function deployContracts(deployer, network, accounts) {
+  // do not deploy testing network
+  if (network === "inprocess_test" || network === "coverage") return;
+
   deployer.then(async () => {
     const lockedAccountAdmin = accounts[1];
     const whitelistAdmin = accounts[2];

package.json

@@ -31,7 +31,7 @@
     "moment": "^2.18.1",
     "solium": "Neufund/Solium#neufund",
     "swarm-js": "^0.1.35",
-    "truffle": "3.4.9",
+    "truffle": "Neufund/truffle#neufund",
     "web3": "0.20.1"
   },
   "scripts": {
@@ -40,14 +40,14 @@
     "lint:eslint": "eslint --ignore-path .gitignore .",
     "lint:solium": "solium --dir contracts --reporter=pretty",
     "testrpc": "./scripts/testrpc.sh",
-    "test": "truffle compile && truffle test --network inprocess",
+    "test": "truffle compile && truffle test --network inprocess_test",
     "test:coverage": "SKIP_GAS_CHECKS=true solidity-coverage && codecov",
     "build": "rm -rf ./build && truffle compile --all",
     "deploy": "truffle migrate --reset --compile-all --network localhost"
   },
   "devDependencies": {
     "eslint-plugin-mocha": "^4.11.0",
     "prettier": "^1.5.3",
-    "solidity-coverage": "^0.2.1"
+    "solidity-coverage": "Neufund/solidity-coverage#neufund"
   }
 }

scripts/test.sh

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+# to be run via yarn test inprocess
+
+code=0
+for f in ./test/*.js
+do
+  if [[ "$f" != *setup.js ]]
+  then
+    echo $f
+    yarn truffle test $f ./test/setup.js --network $1
+    code=$(($code + $?))
+  fi
+done
+exit $code

test/AccessControl.js

@@ -3,32 +3,23 @@ import EvmError from "./helpers/EVMThrow";
 import { eventValue } from "./helpers/events";
 import { TriState } from "./helpers/triState";
 import roles from "./helpers/roles";
-import { saveBlockchain, restoreBlockchain } from "./helpers/evmCommands";
 
 const RoleBasedAccessControl = artifacts.require("RoleBasedAccessControl");
 const TestAccessControlTruffleMixin = artifacts.require(
   "TestAccessControlTruffleMixin"
 );
 
 contract("AccessControl", ([accessController, owner1, owner2]) => {
-  let snapshot;
   let accessControl;
   let accessControlled;
   let exampleRole;
 
-  before(async () => {
+  beforeEach(async () => {
     accessControl = await RoleBasedAccessControl.new();
     accessControlled = await TestAccessControlTruffleMixin.new(
       accessControl.address
     );
-
     exampleRole = roles.example;
-    snapshot = await saveBlockchain();
-  });
-
-  beforeEach(async () => {
-    await restoreBlockchain(snapshot);
-    snapshot = await saveBlockchain();
   });
 
   function expectAccessChangedEvent(

test/Agreement.js

@@ -4,23 +4,18 @@ import { deployControlContracts } from "./helpers/deployContracts";
 import EvmError from "./helpers/EVMThrow";
 import { TriState } from "./helpers/triState";
 import roles from "./helpers/roles";
-import {
-  promisify,
-  saveBlockchain,
-  restoreBlockchain
-} from "./helpers/evmCommands";
+import { promisify } from "./helpers/evmCommands";
 
 const TestAgreement = artifacts.require("TestAgreement");
 
 contract(
   "Agreement",
   ([_, platformOperatorRepresentative, signer1, signer2]) => {
-    let snapshot;
     let agreement;
     let accessControl;
     let forkArbiter;
 
-    before(async () => {
+    beforeEach(async () => {
       [accessControl, forkArbiter] = await deployControlContracts();
 
       agreement = await TestAgreement.new(
@@ -33,12 +28,6 @@ contract(
         agreement.address,
         TriState.Allow
       );
-      snapshot = await saveBlockchain();
-    });
-
-    beforeEach(async () => {
-      await restoreBlockchain(snapshot);
-      snapshot = await saveBlockchain();
     });
 
     async function amendAgreement(agreementUri) {

test/Commitment.js

@@ -252,7 +252,7 @@ contract(
           from: other
         });
 
-        expect(tx).to.be.rejectedWith(EvmError);
+        await expect(tx).to.be.rejectedWith(EvmError);
       });
 
       it("should accept whitelist only during Before", async () => {
@@ -1088,7 +1088,7 @@ contract(
         });
         await expect(
           commitment.commitEuro({ from: investor })
-        ).to.to.be.rejectedWith(EvmError);
+        ).to.be.rejectedWith(EvmError);
       });
     });
 
@@ -1440,7 +1440,7 @@ contract(
         await increaseTime(WHITELIST_START);
         await expect(
           commitment.commit({ from: investor, value: MIN_TICKET_ETH.mul(10) })
-        ).to.to.be.rejectedWith(EvmError);
+        ).to.be.rejectedWith(EvmError);
       });
     });
 

test/EtherToken.js

@@ -1,7 +1,6 @@
 import { expect } from "chai";
 import { prettyPrintGasCost } from "./helpers/gasUtils";
 import createAccessPolicy from "./helpers/createAccessPolicy";
-import { saveBlockchain, restoreBlockchain } from "./helpers/evmCommands";
 import {
   basicTokenTests,
   standardTokenTests,
@@ -18,18 +17,11 @@ import { etherToWei } from "./helpers/unitConverter";
 const EtherToken = artifacts.require("EtherToken");
 
 contract("EtherToken", ([broker, ...investors]) => {
-  let snapshot;
   let etherToken;
 
-  before(async () => {
+  beforeEach(async () => {
     const rbac = await createAccessPolicy([]);
     etherToken = await EtherToken.new(rbac.address);
-    snapshot = await saveBlockchain();
-  });
-
-  beforeEach(async () => {
-    await restoreBlockchain(snapshot);
-    snapshot = await saveBlockchain();
   });
 
   describe("specific tests", () => {

test/EthereumForkArbiter.js

@@ -3,31 +3,20 @@ import { prettyPrintGasCost } from "./helpers/gasUtils";
 import createAccessPolicy from "./helpers/createAccessPolicy";
 import { eventValue } from "./helpers/events";
 import roles from "./helpers/roles";
-import {
-  promisify,
-  saveBlockchain,
-  restoreBlockchain
-} from "./helpers/evmCommands";
+import { promisify } from "./helpers/evmCommands";
 
 const EthereumForkArbiter = artifacts.require("EthereumForkArbiter");
 
 contract("EthereumForkArbiter", ([deployer, arbiter, other]) => {
-  let snapshot;
   let ethereumForkArbiter;
   let block;
 
-  before(async () => {
+  beforeEach(async () => {
     const accessPolicy = await createAccessPolicy([
       { subject: arbiter, role: roles.platformOperatorRepresentative }
     ]);
     ethereumForkArbiter = await EthereumForkArbiter.new(accessPolicy.address);
     block = await promisify(web3.eth.getBlock)("latest");
-    snapshot = await saveBlockchain();
-  });
-
-  beforeEach(async () => {
-    await restoreBlockchain(snapshot);
-    snapshot = await saveBlockchain();
   });
 
   it("should deploy", async () => {

test/EuroToken.js

@@ -1,7 +1,6 @@
 import { expect } from "chai";
 import { prettyPrintGasCost } from "./helpers/gasUtils";
 import createAccessPolicy from "./helpers/createAccessPolicy";
-import { saveBlockchain, restoreBlockchain } from "./helpers/evmCommands";
 import {
   basicTokenTests,
   standardTokenTests,
@@ -19,21 +18,14 @@ import EvmError from "./helpers/EVMThrow";
 const EuroToken = artifacts.require("EuroToken");
 
 contract("EuroToken", ([_, depositManager, other, broker, ...investors]) => {
-  let snapshot;
   let rbac;
   let euroToken;
 
-  before(async () => {
+  beforeEach(async () => {
     rbac = await createAccessPolicy([
       { subject: depositManager, role: roles.eurtDepositManager }
     ]);
     euroToken = await EuroToken.new(rbac.address);
-    snapshot = await saveBlockchain();
-  });
-
-  beforeEach(async () => {
-    await restoreBlockchain(snapshot);
-    snapshot = await saveBlockchain();
   });
 
   describe("specific tests", () => {