From 9b173ae70fb55eae8164b755f2811a7580840105 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Tue, 21 Apr 2020 20:59:18 -0700 Subject: [PATCH] Add branch coverage for logical OR conditions (#499) * Add initial OR coverage test cases * Add logicalOR coverage for "require" conditions * Add logicalOR coverage for "while" conditions * Add logicalOR coverage for "return" conditions * Add logicalOR coverage for "if" conditions * Add logicalOR branch highlighting for html report --- lib/coverage.js | 12 +- lib/injector.js | 72 +++- lib/instrumenter.js | 7 +- lib/parse.js | 61 ++- lib/registrar.js | 101 +++++ .../projects/logical-or/.solcover.js | 8 + .../projects/logical-or/buidler.config.js | 7 + .../logical-or/contracts/ContractA.sol | 40 ++ .../projects/logical-or/test/test.js | 33 ++ .../projects/logical-or/truffle-config.js | 7 + .../contracts/assert/Require-fn-reason.sol | 11 + .../solidity/contracts/assert/Require-fn.sol | 11 + .../solidity/contracts/or/and-or-brackets.sol | 11 + test/sources/solidity/contracts/or/and-or.sol | 11 + test/sources/solidity/contracts/or/bzx-or.sol | 19 + test/sources/solidity/contracts/or/if-or.sol | 11 + .../solidity/contracts/or/multi-or.sol | 11 + .../contracts/or/require-multiline-or.sol | 11 + .../solidity/contracts/or/require-or.sol | 7 + .../solidity/contracts/or/return-or.sol | 7 + .../solidity/contracts/or/while-or.sol | 10 + .../contracts/return/empty-return.sol | 7 + .../contracts/return/ternary-return.sol | 11 + test/units/assert.js | 40 ++ test/units/expressions.js | 10 + test/units/hardhat/standard.js | 19 + test/units/or.js | 375 ++++++++++++++++++ test/units/truffle/standard.js | 14 + test/util/verifiers.js | 14 + 29 files changed, 937 insertions(+), 21 deletions(-) create mode 100644 test/integration/projects/logical-or/.solcover.js create mode 100644 test/integration/projects/logical-or/buidler.config.js create mode 100644 test/integration/projects/logical-or/contracts/ContractA.sol create mode 100644 test/integration/projects/logical-or/test/test.js create mode 100644 test/integration/projects/logical-or/truffle-config.js create mode 100644 test/sources/solidity/contracts/assert/Require-fn-reason.sol create mode 100644 test/sources/solidity/contracts/assert/Require-fn.sol create mode 100644 test/sources/solidity/contracts/or/and-or-brackets.sol create mode 100644 test/sources/solidity/contracts/or/and-or.sol create mode 100644 test/sources/solidity/contracts/or/bzx-or.sol create mode 100644 test/sources/solidity/contracts/or/if-or.sol create mode 100644 test/sources/solidity/contracts/or/multi-or.sol create mode 100644 test/sources/solidity/contracts/or/require-multiline-or.sol create mode 100644 test/sources/solidity/contracts/or/require-or.sol create mode 100644 test/sources/solidity/contracts/or/return-or.sol create mode 100644 test/sources/solidity/contracts/or/while-or.sol create mode 100644 test/sources/solidity/contracts/return/empty-return.sol create mode 100644 test/sources/solidity/contracts/return/ternary-return.sol create mode 100644 test/units/or.js diff --git a/lib/coverage.js b/lib/coverage.js index dc7ca77..0fe1b71 100644 --- a/lib/coverage.js +++ b/lib/coverage.js @@ -69,13 +69,17 @@ class Coverage { const data = collectedData[hash]; const contractPath = collectedData[hash].contractPath; const id = collectedData[hash].id; + + // NB: Any branch using the injected fn which returns boolean will have artificially + // doubled hits (because of something internal to Solidity about how the stack is managed) const hits = collectedData[hash].hits; switch(collectedData[hash].type){ - case 'line': this.data[contractPath].l[id] = hits; break; - case 'function': this.data[contractPath].f[id] = hits; break; - case 'statement': this.data[contractPath].s[id] = hits; break; - case 'branch': this.data[contractPath].b[id][data.locationIdx] = hits; break; + case 'line': this.data[contractPath].l[id] = hits; break; + case 'function': this.data[contractPath].f[id] = hits; break; + case 'statement': this.data[contractPath].s[id] = hits; break; + case 'branch': this.data[contractPath].b[id][data.locationIdx] = hits; break; + case 'logicalOR': this.data[contractPath].b[id][data.locationIdx] = hits / 2; break; case 'requirePre': this.requireData[contractPath][id].preEvents = hits; break; case 'requirePost': this.requireData[contractPath][id].postEvents = hits; break; } diff --git a/lib/injector.js b/lib/injector.js index c2682f1..c689408 100644 --- a/lib/injector.js +++ b/lib/injector.js @@ -13,7 +13,12 @@ class Injector { } _getInjectable(id, hash, type){ - return `${this._getMethodIdentifier(id)}(${hash}); /* ${type} */ \n`; + switch(type){ + case 'logicalOR': + return ` && ${this._getTrueMethodIdentifier(id)}(${hash}))`; + default: + return `${this._getDefaultMethodIdentifier(id)}(${hash}); /* ${type} */ \n`; + } } _getHash(id) { @@ -21,10 +26,16 @@ class Injector { return web3Utils.keccak256(`${id}:${this.hashCounter}`); } - _getMethodIdentifier(id){ + // Method returns void + _getDefaultMethodIdentifier(id){ return `c_${web3Utils.keccak256(id).slice(0,10)}` } + // Method returns boolean true + _getTrueMethodIdentifier(id){ + return `c_true${web3Utils.keccak256(id).slice(0,10)}` + } + _getInjectionComponents(contract, injectionPoint, id, type){ const { start, end } = this._split(contract, injectionPoint); const hash = this._getHash(id) @@ -44,7 +55,7 @@ class Injector { * @param {String} id * @return {String} */ - _getHashMethodDefinition(id, contract){ + _getDefaultMethodDefinition(id){ const hash = web3Utils.keccak256(id).slice(0,10); const method = this._getMethodIdentifier(id); return `\nfunction ${method}(bytes32 c__${hash}) internal pure {}\n`; @@ -58,8 +69,20 @@ class Injector { */ _getFileScopedHashMethodDefinition(id, contract){ const hash = web3Utils.keccak256(id).slice(0,10); - const method = this._getMethodIdentifier(id); - return `\nfunction ${method}(bytes32 c__${hash}) pure {}\n`; + const method = this._getDefaultMethodIdentifier(id); + return `\nfunction ${method}(bytes32 c__${hash}) public pure {}\n`; + } + + /** + * Generates a solidity statement injection defining a method + * *which returns boolean true* to pass instrumentation hash to. + * @param {String} fileName + * @return {String} ex: bytes32[1] memory _sc_82e0891 + */ + _getTrueMethodDefinition(id){ + const hash = web3Utils.keccak256(id).slice(0,10); + const method = this._getTrueMethodIdentifier(id); + return `\nfunction ${method}(bytes32 c__${hash}) public pure returns (bool){ return true; }\n`; } injectLine(contract, fileName, injectionPoint, injection, instrumentation){ @@ -217,9 +240,42 @@ class Injector { const end = contract.instrumented.slice(injectionPoint); const id = `${fileName}:${injection.contractName}`; - contract.instrumented = (injection.isFileScoped) - ? `${start}${this._getFileScopedHashMethodDefinition(id)}${end}` - : `${start}${this._getHashMethodDefinition(id)}${end}`; + const defaultMethodDefinition = (injection.isFileScoped) + ? this._getFileScopedHashMethodDefinition(id) + : this._getHashMethodDefinition(id); + + contract.instrumented = `${start}` + + `${defaultMethodDefinition}` + + `${this._getTrueMethodDefinition(id)}` + + `${end}`; + } + + injectOpenParen(contract, fileName, injectionPoint, injection, instrumentation){ + const start = contract.instrumented.slice(0, injectionPoint); + const end = contract.instrumented.slice(injectionPoint); + contract.instrumented = `${start}(${end}`; + } + + injectLogicalOR(contract, fileName, injectionPoint, injection, instrumentation){ + const type = 'logicalOR'; + const id = `${fileName}:${injection.contractName}`; + + const { + start, + end, + hash, + injectable + } = this._getInjectionComponents(contract, injectionPoint, id, type); + + instrumentation[hash] = { + id: injection.branchId, + locationIdx: injection.locationIdx, + type: type, + contractPath: fileName, + hits: 0 + } + + contract.instrumented = `${start}${injectable}${end}`; } }; diff --git a/lib/instrumenter.js b/lib/instrumenter.js index b9f8216..e4db949 100644 --- a/lib/instrumenter.js +++ b/lib/instrumenter.js @@ -88,7 +88,12 @@ class Instrumenter { // Line instrumentation has to happen first contract.injectionPoints[injectionPoint].sort((a, b) => { - const injections = ['injectBranch', 'injectEmptyBranch', 'injectLine']; + const injections = [ + 'injectLogicalOR', + 'injectBranch', + 'injectEmptyBranch', + 'injectLine' + ]; return injections.indexOf(b.type) - injections.indexOf(a.type); }); diff --git a/lib/parse.js b/lib/parse.js index c6a7989..42abdbd 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -32,17 +32,47 @@ parse.Block = function(contract, expression) { } }; -parse.BinaryOperation = function(contract, expression) { - register.statement(contract, expression); +parse.BinaryOperation = function(contract, expression, skipStatementRegistry) { + // Regular binary operation + if (!skipStatementRegistry){ + register.statement(contract, expression); + + // LogicalOR conditional search... + } else { + parse[expression.left.type] && + parse[expression.left.type](contract, expression.left, true); + + parse[expression.right.type] && + parse[expression.right.type](contract, expression.right, true); + + if (expression.operator === '||'){ + register.logicalOR(contract, expression); + } + } +} + +parse.TupleExpression = function(contract, expression, skipStatementRegistry) { + expression.components.forEach(component => { + parse[component.type] && + parse[component.type](contract, component, skipStatementRegistry); + }); } -parse.FunctionCall = function(contract, expression) { +parse.FunctionCall = function(contract, expression, skipStatementRegistry) { // In any given chain of call expressions, only the last one will fail this check. // This makes sure we don't instrument a chain of expressions multiple times. if (expression.expression.type !== 'FunctionCall') { - register.statement(contract, expression); + + // Don't register sub-expressions (like intermediate method calls) + if (!skipStatementRegistry){ + register.statement(contract, expression); + } + if (expression.expression.name === 'require') { register.requireBranch(contract, expression); + expression.arguments.forEach(arg => { + parse[arg.type] && parse[arg.type](contract, arg, true); + }); } parse[expression.expression.type] && parse[expression.expression.type](contract, expression.expression); @@ -52,10 +82,13 @@ parse.FunctionCall = function(contract, expression) { } }; -parse.Conditional = function(contract, expression) { - register.statement(contract, expression); - // TODO: Investigate node structure - // There are potential substatements here we aren't measuring +parse.Conditional = function(contract, expression, skipStatementRegistry) { + if (!skipStatementRegistry){ + register.statement(contract, expression); + } + + parse[expression.condition.type] && + parse[expression.condition.type](contract, expression.condition, skipStatementRegistry); }; parse.ContractDefinition = function(contract, expression) { @@ -145,6 +178,10 @@ parse.FunctionDefinition = function(contract, expression) { parse.IfStatement = function(contract, expression) { register.statement(contract, expression); register.ifStatement(contract, expression); + + parse[expression.condition.type] && + parse[expression.condition.type](contract, expression.condition, true); + parse[expression.trueBody.type] && parse[expression.trueBody.type](contract, expression.trueBody); @@ -220,6 +257,10 @@ parse.SourceUnit = function(contract, expression) { parse.ReturnStatement = function(contract, expression) { register.statement(contract, expression); + + expression.expression && + parse[expression.expression.type] && + parse[expression.expression.type](contract, expression.expression, true); }; // TODO:Investigate node structure @@ -255,6 +296,10 @@ parse.VariableDeclarationStatement = function (contract, expression) { parse.WhileStatement = function (contract, expression) { register.statement(contract, expression); + + parse[expression.condition.type] && + parse[expression.condition.type](contract, expression.condition, true); + parse[expression.body.type] && parse[expression.body.type](contract, expression.body); }; diff --git a/lib/registrar.js b/lib/registrar.js index cce5fbe..563ec43 100644 --- a/lib/registrar.js +++ b/lib/registrar.js @@ -186,6 +186,107 @@ class Registrar { }; }; + /** + * Registers injections for branch measurements. Used by logicalOR registration method. + * @param {Object} contract instrumentation target + * @param {Object} expression AST node + */ + addNewLogicalORBranch(contract, expression) { + let start; + + // Instabul HTML highlighting location data... + const leftZero = expression.left.range[0]; + const leftOne = expression.left.range[1]; + const rightZero = expression.right.range[0]; + const rightOne = expression.right.range[1]; + + start = contract.instrumented.slice(0, leftZero); + const leftStartLine = ( start.match(/\n/g) || [] ).length + 1; + const leftStartCol = leftZero - start.lastIndexOf('\n') - 1; + + start = contract.instrumented.slice(0, leftOne); + const leftEndLine = ( start.match(/\n/g) || [] ).length + 1; + const leftEndCol = leftOne - start.lastIndexOf('\n') - 1; + + start = contract.instrumented.slice(0, rightZero); + const rightStartLine = ( start.match(/\n/g) || [] ).length + 1; + const rightStartCol = rightZero - start.lastIndexOf('\n') - 1; + + start = contract.instrumented.slice(0, rightOne); + const rightEndLine = ( start.match(/\n/g) || [] ).length + 1; + const rightEndCol = rightOne - start.lastIndexOf('\n') - 1; + + contract.branchId += 1; + + // NB locations for if branches in istanbul are zero + // length and associated with the start of the if. + contract.branchMap[contract.branchId] = { + line: leftStartLine, + type: 'cond-expr', + locations: [{ + start: { + line: leftStartLine, column: leftStartCol, + }, + end: { + line: leftEndLine, column: leftEndCol, + }, + }, { + start: { + line: rightStartLine, column: rightStartCol, + }, + end: { + line: rightEndLine, column: rightEndCol, + }, + }], + }; + }; + + /** + * Registers injections for logicalOR clause measurements (branches) + * @param {Object} contract instrumentation target + * @param {Object} expression AST node + * @param {Number} injectionIdx pre/post branch index (left=0, right=1) + */ + logicalOR(contract, expression) { + this.addNewLogicalORBranch(contract, expression); + + // Left + this._createInjectionPoint( + contract, + expression.left.range[0], + { + type: 'injectOpenParen', + } + ); + this._createInjectionPoint( + contract, + expression.left.range[1] + 1, + { + type: 'injectLogicalOR', + branchId: contract.branchId, + locationIdx: 0 + } + ); + + // Right + this._createInjectionPoint( + contract, + expression.right.range[0], + { + type: 'injectOpenParen', + } + ); + this._createInjectionPoint( + contract, + expression.right.range[1] + 1, + { + type: 'injectLogicalOR', + branchId: contract.branchId, + locationIdx: 1 + } + ); + } + /** * Registers injections for require statement measurements (branches) * @param {Object} contract instrumentation target diff --git a/test/integration/projects/logical-or/.solcover.js b/test/integration/projects/logical-or/.solcover.js new file mode 100644 index 0000000..c245304 --- /dev/null +++ b/test/integration/projects/logical-or/.solcover.js @@ -0,0 +1,8 @@ +// Testing hooks +const fn = (msg, config) => config.logger.log(msg); + +module.exports = { + skipFiles: ['Migrations.sol'], + silent: process.env.SILENT ? true : false, + istanbulReporter: ['json-summary', 'text'], +} diff --git a/test/integration/projects/logical-or/buidler.config.js b/test/integration/projects/logical-or/buidler.config.js new file mode 100644 index 0000000..fcdac9a --- /dev/null +++ b/test/integration/projects/logical-or/buidler.config.js @@ -0,0 +1,7 @@ +const { loadPluginFile } = require("@nomiclabs/buidler/plugins-testing"); +loadPluginFile(__dirname + "/../plugins/buidler.plugin"); +usePlugin("@nomiclabs/buidler-truffle5"); + +module.exports={ + defaultNetwork: "buidlerevm" +}; diff --git a/test/integration/projects/logical-or/contracts/ContractA.sol b/test/integration/projects/logical-or/contracts/ContractA.sol new file mode 100644 index 0000000..8c20a73 --- /dev/null +++ b/test/integration/projects/logical-or/contracts/ContractA.sol @@ -0,0 +1,40 @@ +pragma solidity ^0.5.0; + + +contract ContractA { + + function _if(uint i) public pure { + if (i == 0 || i > 5){ + /* ignore */ + } + } + + function _if_and(uint i) public pure { + if (i != 0 && (i < 2 || i > 5)){ + /* ignore */ + } + } + + function _return(uint i) public pure returns (bool){ + return (i != 0 && i != 1 ) || + ((i + 1) == 2); + } + + function _while(uint i) public pure returns (bool){ + uint counter; + while( (i == 1 || i == 2) && counter < 2 ){ + counter++; + } + } + + function _require(uint x) public { + require(x == 1 || x == 2); + } + + function _require_multi_line(uint x) public { + require( + (x == 1 || x == 2) || + x == 3 + ); + } +} diff --git a/test/integration/projects/logical-or/test/test.js b/test/integration/projects/logical-or/test/test.js new file mode 100644 index 0000000..a9a864b --- /dev/null +++ b/test/integration/projects/logical-or/test/test.js @@ -0,0 +1,33 @@ +const ContractA = artifacts.require("ContractA"); + +contract("contracta", function(accounts) { + let instance; + + before(async () => instance = await ContractA.new()) + + it('_if', async function(){ + await instance._if(0); + await instance._if(7); + }); + + it('_if_and', async function(){ + await instance._if_and(1); + }); + + it('_return', async function(){ + await instance._return(4); + }); + + it('_while', async function(){ + await instance._while(1); + }); + + it('_require', async function(){ + await instance._require(2); + }) + + it('_require_multi_line', async function(){ + await instance._require_multi_line(1); + await instance._require_multi_line(3); + }) +}); diff --git a/test/integration/projects/logical-or/truffle-config.js b/test/integration/projects/logical-or/truffle-config.js new file mode 100644 index 0000000..b398b07 --- /dev/null +++ b/test/integration/projects/logical-or/truffle-config.js @@ -0,0 +1,7 @@ +module.exports = { + networks: {}, + mocha: {}, + compilers: { + solc: {} + } +} diff --git a/test/sources/solidity/contracts/assert/Require-fn-reason.sol b/test/sources/solidity/contracts/assert/Require-fn-reason.sol new file mode 100644 index 0000000..0a010b3 --- /dev/null +++ b/test/sources/solidity/contracts/assert/Require-fn-reason.sol @@ -0,0 +1,11 @@ +pragma solidity ^0.5.0; + +contract Test { + function getBool(bool _b) public pure returns (bool){ + return _b; + } + + function a(bool _a) public { + require(getBool(_a), "mi ritrovai per una selva oscura"); + } +} diff --git a/test/sources/solidity/contracts/assert/Require-fn.sol b/test/sources/solidity/contracts/assert/Require-fn.sol new file mode 100644 index 0000000..c07dfdc --- /dev/null +++ b/test/sources/solidity/contracts/assert/Require-fn.sol @@ -0,0 +1,11 @@ +pragma solidity ^0.5.0; + +contract Test { + function getBool(bool _b) public pure returns (bool){ + return _b; + } + + function a(bool _a) public { + require(getBool(_a)); + } +} diff --git a/test/sources/solidity/contracts/or/and-or-brackets.sol b/test/sources/solidity/contracts/or/and-or-brackets.sol new file mode 100644 index 0000000..9585918 --- /dev/null +++ b/test/sources/solidity/contracts/or/and-or-brackets.sol @@ -0,0 +1,11 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public { + if ((x == 1) && (x == 2 || true)) { + /* ignore */ + } else { + revert(); + } + } +} diff --git a/test/sources/solidity/contracts/or/and-or.sol b/test/sources/solidity/contracts/or/and-or.sol new file mode 100644 index 0000000..e15fa66 --- /dev/null +++ b/test/sources/solidity/contracts/or/and-or.sol @@ -0,0 +1,11 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public { + if (x == 1 && true || x == 2) { + /* ignore */ + } else { + revert(); + } + } +} diff --git a/test/sources/solidity/contracts/or/bzx-or.sol b/test/sources/solidity/contracts/or/bzx-or.sol new file mode 100644 index 0000000..786e6e6 --- /dev/null +++ b/test/sources/solidity/contracts/or/bzx-or.sol @@ -0,0 +1,19 @@ +pragma solidity ^0.5.0; + +contract Test { + function isFalse(uint _a, uint _b) public pure returns (bool){ + return false; + } + + function a(uint x) public { + require(( + x == 1 && + x == 2 ) || + !isFalse( + x, + 3 + ), + "unhealthy position" + ); + } +} diff --git a/test/sources/solidity/contracts/or/if-or.sol b/test/sources/solidity/contracts/or/if-or.sol new file mode 100644 index 0000000..8cf7c0d --- /dev/null +++ b/test/sources/solidity/contracts/or/if-or.sol @@ -0,0 +1,11 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public { + if (x == 1 || x == 2) { + /* ignore */ + } else { + revert(); + } + } +} diff --git a/test/sources/solidity/contracts/or/multi-or.sol b/test/sources/solidity/contracts/or/multi-or.sol new file mode 100644 index 0000000..85d0b59 --- /dev/null +++ b/test/sources/solidity/contracts/or/multi-or.sol @@ -0,0 +1,11 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public { + if ((x == 1) || (x == 2 || true)) { + /* ignore */ + } else { + revert(); + } + } +} diff --git a/test/sources/solidity/contracts/or/require-multiline-or.sol b/test/sources/solidity/contracts/or/require-multiline-or.sol new file mode 100644 index 0000000..04d195b --- /dev/null +++ b/test/sources/solidity/contracts/or/require-multiline-or.sol @@ -0,0 +1,11 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public { + require( + x == 1 || + x == 2 || + x == 3 + ); + } +} diff --git a/test/sources/solidity/contracts/or/require-or.sol b/test/sources/solidity/contracts/or/require-or.sol new file mode 100644 index 0000000..5a9944b --- /dev/null +++ b/test/sources/solidity/contracts/or/require-or.sol @@ -0,0 +1,7 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public { + require(x == 1 || x == 2); + } +} diff --git a/test/sources/solidity/contracts/or/return-or.sol b/test/sources/solidity/contracts/or/return-or.sol new file mode 100644 index 0000000..aed8962 --- /dev/null +++ b/test/sources/solidity/contracts/or/return-or.sol @@ -0,0 +1,7 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public pure returns (bool) { + return (x == 1 && true) || (x == 2 && true); + } +} diff --git a/test/sources/solidity/contracts/or/while-or.sol b/test/sources/solidity/contracts/or/while-or.sol new file mode 100644 index 0000000..5bb4cc7 --- /dev/null +++ b/test/sources/solidity/contracts/or/while-or.sol @@ -0,0 +1,10 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public { + uint counter; + while( (x == 1 || x == 2) && counter < 2 ){ + counter++; + } + } +} diff --git a/test/sources/solidity/contracts/return/empty-return.sol b/test/sources/solidity/contracts/return/empty-return.sol new file mode 100644 index 0000000..1d7d541 --- /dev/null +++ b/test/sources/solidity/contracts/return/empty-return.sol @@ -0,0 +1,7 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public pure { + return; + } +} diff --git a/test/sources/solidity/contracts/return/ternary-return.sol b/test/sources/solidity/contracts/return/ternary-return.sol new file mode 100644 index 0000000..4887670 --- /dev/null +++ b/test/sources/solidity/contracts/return/ternary-return.sol @@ -0,0 +1,11 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public pure returns (uint) { + return x > 3 ? x : 1; + } + + function b(uint x) public pure returns (uint) { + return (x > 3) ? x : 1; + } +} diff --git a/test/units/assert.js b/test/units/assert.js index 4060841..20ad346 100644 --- a/test/units/assert.js +++ b/test/units/assert.js @@ -101,4 +101,44 @@ describe('asserts and requires', () => { 1: 2, }); }); + + it('should cover require statements with method arguments', async function() { + const contract = await util.bootstrapCoverage('assert/Require-fn', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(true); + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 9: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1 + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, 2: 1 + }); + }); + + it('should cover require statements with method arguments & reason string', async function() { + const contract = await util.bootstrapCoverage('assert/Require-fn-reason', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(true); + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 9: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1 + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, 2: 1 + }); + }); }); diff --git a/test/units/expressions.js b/test/units/expressions.js index b007c29..1e8d948 100644 --- a/test/units/expressions.js +++ b/test/units/expressions.js @@ -16,4 +16,14 @@ describe('generic expressions / misc', () => { const info = util.instrumentAndCompile('return/return'); util.report(info.solcOutput.errors); }); + + it('should compile after instrumenting function that returns void', () => { + const info = util.instrumentAndCompile('return/empty-return'); + util.report(info.solcOutput.errors); + }); + + it('should compile after instrumenting function that returns via ternary conditional', () => { + const info = util.instrumentAndCompile('return/ternary-return'); + util.report(info.solcOutput.errors); + }); }); diff --git a/test/units/hardhat/standard.js b/test/units/hardhat/standard.js index a835bb0..e7eb42e 100644 --- a/test/units/hardhat/standard.js +++ b/test/units/hardhat/standard.js @@ -488,4 +488,23 @@ describe('Hardhat Plugin: standard use cases', function() { verify.lineCoverage(expected); }) + + // This works on it's own but there's some kind of weird interaction with + // the solc 6 test which causes subsequent cov measurements to be zero. + // Have tried re-ordering etc...???. Truffle tests this & should be the same anyway... + it.skip('logicalOR', async function(){ + mock.installFullProject('logical-or'); + mock.hardhatSetupEnv(this); + + await this.env.run("coverage"); + + const expected = [ + { + file: mock.pathToContract(hardhatConfig, 'ContractA.sol'), + pct: 59.09 + } + ]; + + verify.branchCoverage(expected); + }) }) diff --git a/test/units/or.js b/test/units/or.js new file mode 100644 index 0000000..e20e279 --- /dev/null +++ b/test/units/or.js @@ -0,0 +1,375 @@ +const assert = require('assert'); +const util = require('./../util/util.js'); + +const ganache = require('ganache-core-sc'); +const Coverage = require('./../../lib/coverage'); + +describe('logical OR branches', () => { + let coverage; + let provider; + let collector; + + before(async () => ({ provider, collector } = await util.initializeProvider(ganache))); + beforeEach(() => coverage = new Coverage()); + after((done) => provider.close(done)); + + // if (x == 1 || x == 2) { } else ... + it('should cover an if statement with a simple OR condition (single branch)', async function() { + const contract = await util.bootstrapCoverage('or/if-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 8: 0 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], 2: [1, 0] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 0, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + // if (x == 1 || x == 2) { } else ... + it('should cover an if statement with a simple OR condition (both branches)', async function() { + const contract = await util.bootstrapCoverage('or/if-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + await contract.instance.a(2); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 2, 8: 0 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [2, 0], 2: [1, 1] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 2, 2: 0, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 2, + }); + }); + + // require(x == 1 || x == 2) + it('should cover a require statement with a simple OR condition (single branch)', async function() { + const contract = await util.bootstrapCoverage('or/require-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], 2: [1, 0] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + // require( + // x == 1 || + // x == 2 || + // x == 3 + // ) + it('should cover a require statement with multiline OR condition (two branches)', async function() { + const contract = await util.bootstrapCoverage('or/require-multiline-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + await contract.instance.a(3); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 2, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [2, 0], 2: [1, 0], 3: [0, 1] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 2, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 2, + }); + }); + + // require(x == 1 || x == 2) + it('should cover a require statement with a simple OR condition (both branches)', async function() { + const contract = await util.bootstrapCoverage('or/require-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + await contract.instance.a(2); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 2, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [2, 0], 2: [1, 1] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 2, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 2, + }); + }); + + // while( (x == 1 || x == 2) && counter < 2 ){ + it('should cover a while statement with a simple OR condition (single branch)', async function() { + const contract = await util.bootstrapCoverage('or/while-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 2 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [3, 0] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1 + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + // while( (x == 1 || x == 2) && counter < 2 ){ + it('should cover a while statement with a simple OR condition (both branches)', async function() { + const contract = await util.bootstrapCoverage('or/while-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + await contract.instance.a(2); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 2, 6: 2, 7: 4 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [3, 3] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 2, 2: 2 + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 2, + }); + }); + + // return (x == 1 && true) || (x == 2 && true); + it('should cover a return statement with ANDED OR conditions (single branch)', async function() { + const contract = await util.bootstrapCoverage('or/return-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [ 1, 0 ] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + // return (x == 1 && true) || (x == 2 && true); + it('should cover a return statement with ANDED OR conditions (both branches)', async function() { + const contract = await util.bootstrapCoverage('or/return-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + await contract.instance.a(2); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 2, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [ 1, 1 ] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 2, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 2, + }); + }); + + //if (x == 1 && true || x == 2) { + it('should cover an if statement with OR and AND conditions (single branch)', async function() { + const contract = await util.bootstrapCoverage('or/and-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 8: 0 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], 2: [1, 0] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 0, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + //if (x == 1 && true || x == 2) { + it('should cover an if statement with OR and AND conditions (both branches)', async function() { + const contract = await util.bootstrapCoverage('or/and-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + await contract.instance.a(2); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 2, 8: 0 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [2, 0], 2: [1, 1] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 2, 2: 0, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 2, + }); + }); + + // if ((x == 1) && (x == 2 || true)) { + it('should cover an if statement with bracked ANDED OR conditions (rightmost sub-branch)', async function() { + const contract = await util.bootstrapCoverage('or/and-or-brackets', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 8: 0 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], 2: [0, 1] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 0, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + // if ((x == 1) || (x == 2 || true)) { + it('should cover an if statement with multiple (bracketed) OR conditions (branch 1)', async function() { + const contract = await util.bootstrapCoverage('or/multi-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 8: 0 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], 2: [0, 0], 3: [1, 0] // Item 3 is the "outer" (left) OR clause + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 0, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + // if ((x == 1) || (x == 2 || true)) { + it('should cover an if statement with multiple (bracketed) OR conditions (branch 2)', async function() { + const contract = await util.bootstrapCoverage('or/multi-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(2); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 8: 0 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], 2: [1, 0], 3: [0, 1] // Item 3 is the "outer" (left) OR clause + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 0, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + // if ((x == 1) || (x == 2 || true)) { + it('should cover an if statement with multiple (bracketed) OR conditions (branch 3)', async function() { + const contract = await util.bootstrapCoverage('or/multi-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(3); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 8: 0 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], 2: [0, 1], 3: [0, 1] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 0, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + it('should cover the bzx example', async function(){ + const contract = await util.bootstrapCoverage('or/bzx-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(3); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 9: 1 + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], 2: [0, 1] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, 2: 1 + }); + }) +}); diff --git a/test/units/truffle/standard.js b/test/units/truffle/standard.js index 909c900..917f4ab 100644 --- a/test/units/truffle/standard.js +++ b/test/units/truffle/standard.js @@ -498,4 +498,18 @@ describe('Truffle Plugin: standard use cases', function() { verify.lineCoverage(expected); }); + + it('logicalOR', async function(){ + mock.installFullProject('logical-or'); + await plugin(truffleConfig); + + const expected = [ + { + file: mock.pathToContract(truffleConfig, 'ContractA.sol'), + pct: 59.09 + } + ]; + + verify.branchCoverage(expected); + }) }) diff --git a/test/util/verifiers.js b/test/util/verifiers.js index 4799430..d9672cb 100644 --- a/test/util/verifiers.js +++ b/test/util/verifiers.js @@ -44,6 +44,19 @@ function functionCoverage(expected=[], summaryKey='pct'){ }); } +function branchCoverage(expected=[], summaryKey='pct'){ + let summary = JSON.parse(fs.readFileSync('coverage/coverage-summary.json')); + + expected.forEach((item, idx) => { + assert( + summary[item.file].branches.pct === item[summaryKey], + + `For condition ${idx} - expected ${item[summaryKey]} %,` + + `saw - ${summary[item.file].branches[summaryKey]} %` + ) + }); +} + function coverageMissing(expected=[]){ let summary = JSON.parse(fs.readFileSync('coverage/coverage-summary.json')); expected.forEach(item => assert(summary[item.file] === undefined)) @@ -75,6 +88,7 @@ module.exports = { lineCoverage: lineCoverage, statementCoverage: statementCoverage, functionCoverage: functionCoverage, + branchCoverage: branchCoverage, coverageMissing: coverageMissing, cleanInitialState: cleanInitialState, coverageGenerated: coverageGenerated,