From 6c568740cbb69858a591b2d18ee8647d552e09d4 Mon Sep 17 00:00:00 2001 From: Facu Spagnuolo Date: Tue, 25 Aug 2020 16:46:58 -0300 Subject: [PATCH] Tests: Fix solidity tests (#616) * Chore: Bump v5.0.0-rc.0 * Tests: Fix solidity tests * Tests: rename test file * Tests: remove useless arg from solidity test assert fn * docs: update coverage badge to be codecov * chore: remove coveralls from package.json Co-authored-by: Brett Sun --- .gitignore | 1 + contracts/test/tests/TestDelegateProxy.sol | 9 ---- .../tests/TestDelegateProxySelfDestruct.sol | 37 ++++++++++++++ package.json | 5 +- readme.md | 4 +- test/contracts/common/delegate_proxy.js | 1 + test/helpers/runSolidityTest.js | 49 ++++++------------- yarn.lock | 31 ++---------- 8 files changed, 64 insertions(+), 73 deletions(-) create mode 100644 contracts/test/tests/TestDelegateProxySelfDestruct.sol diff --git a/.gitignore b/.gitignore index c2c43cee2..d1f63c2a5 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ yarn-error.log package-lock.json # coverage +.coverage* coverage/ coverageEnv/ coverage.json diff --git a/contracts/test/tests/TestDelegateProxy.sol b/contracts/test/tests/TestDelegateProxy.sol index 1ba5a836a..cf75b9db2 100644 --- a/contracts/test/tests/TestDelegateProxy.sol +++ b/contracts/test/tests/TestDelegateProxy.sol @@ -10,7 +10,6 @@ import "../../evmscript/ScriptHelpers.sol"; contract Target { function dontReturn() public pure {} function fail() public pure { revert(); } - function die() public { selfdestruct(0); } } @@ -66,12 +65,4 @@ contract TestDelegateProxy is DelegateProxy { bool result = isContract(nonContract); Assert.isFalse(result, "should return false"); } - - // keep as last test as it will kill this contract - function testDieIfMinReturn0() public { - Assert.isTrue(true, ''); // Make at least one assertion to satisfy the runner - - delegatedFwd(target, target.die.selector.toBytes()); - Assert.fail('should be dead'); - } } diff --git a/contracts/test/tests/TestDelegateProxySelfDestruct.sol b/contracts/test/tests/TestDelegateProxySelfDestruct.sol new file mode 100644 index 000000000..f27b2b134 --- /dev/null +++ b/contracts/test/tests/TestDelegateProxySelfDestruct.sol @@ -0,0 +1,37 @@ +pragma solidity 0.4.24; + +import "../helpers/Assert.sol"; + +import "../../common/DelegateProxy.sol"; +import "../../evmscript/ScriptHelpers.sol"; + + +contract Target { + function die() public { selfdestruct(0); } +} + +contract TestDelegateProxySelfDestruct is DelegateProxy { + using ScriptHelpers for *; + + Target target; + + // Mock ERCProxy implementation + function implementation() public view returns (address) { + return this; + } + + function proxyType() public pure returns (uint256) { + return FORWARDING; + } + + function beforeAll() public { + target = new Target(); + } + + function testDieIfMinReturn0() public { + Assert.isTrue(true, ''); // Make at least one assertion to satisfy the runner + + delegatedFwd(target, target.die.selector.toBytes()); + Assert.fail('should be dead'); + } +} diff --git a/package.json b/package.json index 6bc4c48df..8e178c323 100644 --- a/package.json +++ b/package.json @@ -27,20 +27,19 @@ }, "dependencies": {}, "devDependencies": { - "@aragon/contract-helpers-test": "^0.1.0-rc.3", + "@aragon/contract-helpers-test": "^0.1.0", "@codechecks/client": "^0.1.5", "@nomiclabs/buidler": "^1.4.3", "@nomiclabs/buidler-ganache": "^1.3.3", "@nomiclabs/buidler-truffle5": "^1.3.4", "@nomiclabs/buidler-web3": "^1.3.4", + "buidler-extract": "^1.0.0", "buidler-gas-reporter": "^0.1.3", "chai": "^4.2.0", - "coveralls": "^3.0.9", "eth-ens-namehash": "^2.0.8", "ethereumjs-abi": "^0.6.5", "solidity-coverage": "^0.7.9", "solium": "^1.2.5", - "buidler-extract": "^1.0.0", "web3": "^1.2.11" } } diff --git a/readme.md b/readme.md index b824cf3ea..91c9ff0ca 100644 --- a/readme.md +++ b/readme.md @@ -10,8 +10,8 @@ Security - - Coverage + + diff --git a/test/contracts/common/delegate_proxy.js b/test/contracts/common/delegate_proxy.js index 9f4f31031..3789b024c 100644 --- a/test/contracts/common/delegate_proxy.js +++ b/test/contracts/common/delegate_proxy.js @@ -1,3 +1,4 @@ const runSolidityTest = require('../../helpers/runSolidityTest') runSolidityTest('TestDelegateProxy') +runSolidityTest('TestDelegateProxySelfDestruct') diff --git a/test/helpers/runSolidityTest.js b/test/helpers/runSolidityTest.js index 5ce0c2d8d..2937a10de 100644 --- a/test/helpers/runSolidityTest.js +++ b/test/helpers/runSolidityTest.js @@ -1,4 +1,4 @@ -const { decodeEvents } = require('@aragon/contract-helpers-test') +const { getEventAt } = require('@aragon/contract-helpers-test') const ASSERT_LIB_EVENTS_ABI = [ { @@ -27,24 +27,8 @@ const HOOKS_MAP = { afterAll: 'afterAll', } -const processResult = (txReceipt, mustAssert) => { - if (!txReceipt || !txReceipt.receipt) { - return - } - const decodedLogs = decodeEvents(txReceipt.receipt, ASSERT_LIB_EVENTS_ABI, 'TestEvent') - decodedLogs.forEach(log => { - if (log.event === 'TestEvent' && log.args.result !== true) { - throw new Error(log.args.message) - } - }) - if (mustAssert && !decodedLogs.length) { - throw new Error('No assertions made') - } -} - /* - * Deploy and link `libName` to provided contract artifact. - * Modifies bytecode in place + * Deploy and link `libName` to provided contract artifact */ const linkLib = async (contract, libName) => { const library = await artifacts.require(libName).new() @@ -55,34 +39,33 @@ const linkLib = async (contract, libName) => { * Runs a solidity test file, via javascript. * Required to smooth over some technical problems in solidity-coverage * - * @param {string} c Name of solidity test file + * @param {string} testContract Name of solidity test file */ -function runSolidityTest(c, mochaContext) { - const artifact = artifacts.require(c) - contract(c, accounts => { +function runSolidityTest(testContract, mochaContext) { + const artifact = artifacts.require(testContract) + + contract(testContract, () => { let deployed before(async () => { await linkLib(artifact, 'Assert') - deployed = await artifact.new() }) + const assertResult = async call => { + const receipt = await call() + const { args: { result, message } } = getEventAt(receipt, 'TestEvent', { decodeForAbi: ASSERT_LIB_EVENTS_ABI }) + if (!result) throw new Error(message || 'No assertions made') + } + mochaContext('> Solidity test', () => { artifact.abi.forEach(interface => { if (interface.type === 'function') { + // Set up hooks if (['beforeAll', 'beforeEach', 'afterEach', 'afterAll'].includes(interface.name)) { - // Set up hooks - global[HOOKS_MAP[interface.name]](async () => { - const receipt = await deployed[interface.name]() - processResult(receipt, false) - }) + global[HOOKS_MAP[interface.name]](async () => await deployed[interface.name]()) } else if (interface.name.startsWith('test')) { - it(interface.name, async () => { - const { tx } = await deployed[interface.name]() - const receipt = await web3.eth.getTransactionReceipt(tx) - processResult(receipt, true) - }) + it(interface.name, async () => await assertResult(deployed[interface.name])) } } }) diff --git a/yarn.lock b/yarn.lock index 7055c2fbb..78c632206 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,10 +2,10 @@ # yarn lockfile v1 -"@aragon/contract-helpers-test@^0.1.0-rc.3": - version "0.1.0-rc.3" - resolved "https://registry.yarnpkg.com/@aragon/contract-helpers-test/-/contract-helpers-test-0.1.0-rc.3.tgz#dec428b5f0f7ffdf851663fd308dbc890ee045c1" - integrity sha512-9bCQagWrrZqaM8pSZq2YAY/uavbuNeaiUnKILFXuVwrVTeX2tjExIcaVTyeiGzZDF/L52ruOMi/o+Ns+T2U/gQ== +"@aragon/contract-helpers-test@^0.1.0": + version "0.1.0" + resolved "https://registry.yarnpkg.com/@aragon/contract-helpers-test/-/contract-helpers-test-0.1.0.tgz#5c5f09739a0b33ab66843bc4849cb2b997d88af0" + integrity sha512-xP2CqqP0Jw/6Jdo1mRg4OxL+3gmsCYV3EJRy7xN8xUrhQIqkOyRptC44X951O7Cr+VeqXJq22rpZSr01TJZhNg== dependencies: web3-eth-abi "1.2.5" web3-utils "1.2.5" @@ -2445,17 +2445,6 @@ cors@^2.8.1: object-assign "^4" vary "^1" -coveralls@^3.0.9: - version "3.1.0" - resolved "https://registry.yarnpkg.com/coveralls/-/coveralls-3.1.0.tgz#13c754d5e7a2dd8b44fe5269e21ca394fb4d615b" - integrity sha512-sHxOu2ELzW8/NC1UP5XVLbZDzO4S3VxfFye3XYCznopHy02YjNkHcj5bKaVw2O7hVaBdBjEdQGpie4II1mWhuQ== - dependencies: - js-yaml "^3.13.1" - lcov-parse "^1.0.0" - log-driver "^1.2.7" - minimist "^1.2.5" - request "^2.88.2" - create-ecdh@^4.0.0: version "4.0.4" resolved "https://registry.yarnpkg.com/create-ecdh/-/create-ecdh-4.0.4.tgz#d6e7f4bffa66736085a0762fd3a632684dabcc4e" @@ -5670,11 +5659,6 @@ lcid@^2.0.0: dependencies: invert-kv "^2.0.0" -lcov-parse@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/lcov-parse/-/lcov-parse-1.0.0.tgz#eb0d46b54111ebc561acb4c408ef9363bdc8f7e0" - integrity sha1-6w1GtUER68VhrLTECO+TY73I9+A= - lead@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/lead/-/lead-1.0.0.tgz#6f14f99a37be3a9dd784f5495690e5903466ee42" @@ -5891,11 +5875,6 @@ lodash@^4.17.4: resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.20.tgz#b44a9b6297bcb698f1c51a3545a2b3b368d59c52" integrity sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA== -log-driver@^1.2.7: - version "1.2.7" - resolved "https://registry.yarnpkg.com/log-driver/-/log-driver-1.2.7.tgz#63b95021f0702fedfa2c9bb0a24e7797d71871d8" - integrity sha512-U7KCmLdqsGHBLeWqYlFA0V0Sl6P08EE1ZrmA9cxjUE0WVqT9qnyVDPz1kzpFEP0jdJuFnasWIfSd7fsaNXkpbg== - log-symbols@3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/log-symbols/-/log-symbols-3.0.0.tgz#f3a08516a5dea893336a7dee14d18a1cfdab77c4" @@ -7620,7 +7599,7 @@ request-promise@^4.2.2: stealthy-require "^1.1.1" tough-cookie "^2.3.3" -request@^2.79.0, request@^2.85.0, request@^2.88.0, request@^2.88.2: +request@^2.79.0, request@^2.85.0, request@^2.88.0: version "2.88.2" resolved "https://registry.yarnpkg.com/request/-/request-2.88.2.tgz#d73c918731cb5a87da047e207234146f664d12b3" integrity sha512-MsvtOrfG9ZcrOwAW+Qi+F6HbD0CWXEh9ou77uOb7FM2WPhwT7smM833PzanhJLsgXjN89Ir6V2PczXNnMpwKhw==