diff --git a/.solcover.js b/.solcover.js index 38a69357e..706360613 100644 --- a/.solcover.js +++ b/.solcover.js @@ -3,7 +3,6 @@ const skipFiles = [ 'test', 'acl/ACLSyntaxSugar.sol', 'common/DepositableStorage.sol', // Used in tests that send ETH - 'common/SafeERC20.sol', // solidity-coverage fails on assembly if (https://github.com/sc-forks/solidity-coverage/issues/287) 'common/UnstructuredStorage.sol' // Used in tests that send ETH ] diff --git a/package.json b/package.json index ca57aac09..5bea6fdc1 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "ethereumjs-abi": "^0.6.5", "ganache-cli": "^6.4.2", "mocha-lcov-reporter": "^1.3.0", - "solidity-coverage": "0.5.8", + "solidity-coverage": "^0.6.2", "solium": "^1.2.3", "truffle": "4.1.14", "truffle-bytecode-manager": "^1.1.1", diff --git a/test/contracts/apps/app_funds.js b/test/contracts/apps/app_funds.js index 623736272..f94433287 100644 --- a/test/contracts/apps/app_funds.js +++ b/test/contracts/apps/app_funds.js @@ -1,7 +1,7 @@ const { hash } = require('eth-ens-namehash') const { onlyIf } = require('../../helpers/onlyIf') -const { getBalance } = require('../../helpers/web3') const { assertRevert } = require('../../helpers/assertThrow') +const { getBalance } = require('../../helpers/web3') const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') diff --git a/test/contracts/apps/recovery_to_vault.js b/test/contracts/apps/recovery_to_vault.js index 6f5e2010c..626bc67de 100644 --- a/test/contracts/apps/recovery_to_vault.js +++ b/test/contracts/apps/recovery_to_vault.js @@ -1,9 +1,8 @@ const { hash } = require('eth-ens-namehash') -const { getBalance } = require('../../helpers/web3') -const { skipCoverage } = require('../../helpers/coverage') +const { assertAmountOfEvents, assertEvent } = require('../../helpers/assertEvent')(web3) const { assertRevert } = require('../../helpers/assertThrow') const { getNewProxyAddress } = require('../../helpers/events') -const { assertAmountOfEvents, assertEvent } = require('../../helpers/assertEvent')(web3) +const { getBalance } = require('../../helpers/web3') const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') @@ -160,15 +159,6 @@ contract('Recovery to vault', ([permissionsRoot]) => { // Test both the Vault itself and when it's behind a proxy to make sure their recovery behaviours are the same for (const vaultType of ['Vault', 'VaultProxy']) { - const skipCoverageIfVaultProxy = test => { - // The VaultMock isn't instrumented during coverage, but the AppProxy is, and so - // transferring to the fallback fails when we're testing with the proxy. - // Native transfers (either .send() or .transfer()) fail under coverage because they're - // limited to 2.3k gas, and the injected instrumentation from coverage makes these - // operations cost more than that limit. - return vaultType === 'VaultProxy' ? skipCoverage(test) : test - } - context(`> ${vaultType}`, () => { let target, vault @@ -218,9 +208,9 @@ contract('Recovery to vault', ([permissionsRoot]) => { await target.enableDeposits() }) - it('does not recover ETH', skipCoverageIfVaultProxy(async () => + it('does not recover ETH', async () => await recoverEth({ target, vault, shouldFail: true }) - )) + ) it('does not recover tokens', async () => await recoverTokens({ @@ -251,9 +241,9 @@ contract('Recovery to vault', ([permissionsRoot]) => { await assertRevert(target.sendTransaction({ value: 1, data: '0x01', gas: SEND_ETH_GAS })) }) - it('recovers ETH', skipCoverageIfVaultProxy(async () => + it('recovers ETH', async () => await recoverEth({ target, vault }) - )) + ) for (const { title, tokenContract } of tokenTestGroups) { it(`recovers ${title}`, async () => { @@ -289,10 +279,10 @@ contract('Recovery to vault', ([permissionsRoot]) => { target = app }) - it('does not allow recovering ETH', skipCoverageIfVaultProxy(async () => + it('does not allow recovering ETH', async () => // Conditional stub doesnt allow eth recoveries await recoverEth({ target, vault, shouldFail: true }) - )) + ) for (const { title, tokenContract } of tokenTestGroups) { it(`allows recovers ${title}`, async () => { @@ -344,8 +334,8 @@ contract('Recovery to vault', ([permissionsRoot]) => { await kernel.setRecoveryVaultAppId(vaultId) }) - it('recovers ETH from the kernel', skipCoverage(async () => { + it('recovers ETH from the kernel', async () => { await recoverEth({ target: kernel, vault }) - })) + }) }) }) diff --git a/test/contracts/kernel/kernel_funds.js b/test/contracts/kernel/kernel_funds.js index 8a6c044a1..de80d198e 100644 --- a/test/contracts/kernel/kernel_funds.js +++ b/test/contracts/kernel/kernel_funds.js @@ -1,6 +1,6 @@ +const { assertRevert } = require('../../helpers/assertThrow') const { onlyIf } = require('../../helpers/onlyIf') const { getBalance } = require('../../helpers/web3') -const { assertRevert } = require('../../helpers/assertThrow') const ACL = artifacts.require('ACL') const Kernel = artifacts.require('Kernel') diff --git a/test/helpers/assertThrow.js b/test/helpers/assertThrow.js index c15565328..e70f6c26e 100644 --- a/test/helpers/assertThrow.js +++ b/test/helpers/assertThrow.js @@ -31,7 +31,7 @@ module.exports = { error.reason = error.message.replace(errorPrefix, '').trim() } - if (process.env.SOLIDITY_COVERAGE !== 'true' && reason) { + if (reason) { assert.equal(error.reason, reason, `Expected revert reason "${reason}" but failed with "${error.reason || 'no reason'}" instead.`) } }, diff --git a/test/helpers/coverage.js b/test/helpers/coverage.js deleted file mode 100644 index f5f313242..000000000 --- a/test/helpers/coverage.js +++ /dev/null @@ -1,14 +0,0 @@ -const skipCoverage = test => { - // Required dynamic this binding to attach onto the running test - return function skipCoverage() { - if (process.env.SOLIDITY_COVERAGE === 'true') { - this.skip() - } else { - return test() - } - } -} - -module.exports = { - skipCoverage -}