diff --git a/lib/coverage.js b/lib/coverage.js index 8d5d6a66..dc7ca779 100644 --- a/lib/coverage.js +++ b/lib/coverage.js @@ -9,7 +9,7 @@ class Coverage { constructor() { this.data = {}; - this.assertData = {}; + this.requireData = {}; } /** @@ -30,7 +30,7 @@ class Coverage { statementMap: {}, branchMap: {}, }; - this.assertData[contractPath] = { }; + this.requireData[contractPath] = { }; info.runnableLines.forEach((item, idx) => { this.data[contractPath].l[info.runnableLines[idx]] = 0; @@ -44,7 +44,7 @@ class Coverage { this.data[contractPath].branchMap = info.branchMap; for (let x = 1; x <= Object.keys(info.branchMap).length; x++) { this.data[contractPath].b[x] = [0, 0]; - this.assertData[contractPath][x] = { + this.requireData[contractPath][x] = { preEvents: 0, postEvents: 0, }; @@ -76,19 +76,19 @@ class Coverage { 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 'assertPre': this.assertData[contractPath][id].preEvents = hits; break; - case 'assertPost': this.assertData[contractPath][id].postEvents = hits; break; + case 'requirePre': this.requireData[contractPath][id].preEvents = hits; break; + case 'requirePost': this.requireData[contractPath][id].postEvents = hits; break; } } // Finally, interpret the assert pre/post events - const contractPaths = Object.keys(this.assertData); + const contractPaths = Object.keys(this.requireData); for (let contractPath of contractPaths){ const contract = this.data[contractPath]; for (let i = 1; i <= Object.keys(contract.b).length; i++) { - const branch = this.assertData[contractPath][i]; + const branch = this.requireData[contractPath][i]; // Was it an assert branch? if (branch && branch.preEvents > 0){ diff --git a/lib/injector.js b/lib/injector.js index e0d4e6dc..1ce313cf 100644 --- a/lib/injector.js +++ b/lib/injector.js @@ -158,8 +158,8 @@ class Injector { contract.instrumented = `${start}else { ${injectable}}${end}`; } - injectAssertPre(contract, fileName, injectionPoint, injection, instrumentation) { - const type = 'assertPre'; + injectRequirePre(contract, fileName, injectionPoint, injection, instrumentation) { + const type = 'requirePre'; const id = `${fileName}:${injection.contractName}`; const { @@ -179,8 +179,8 @@ class Injector { contract.instrumented = `${start}${injectable}${end}`; } - injectAssertPost(contract, fileName, injectionPoint, injection, instrumentation) { - const type = 'assertPost'; + injectRequirePost(contract, fileName, injectionPoint, injection, instrumentation) { + const type = 'requirePost'; const id = `${fileName}:${injection.contractName}`; const { diff --git a/lib/parse.js b/lib/parse.js index faa50987..039d347a 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -39,8 +39,8 @@ parse.FunctionCall = function(contract, expression) { // This makes sure we don't instrument a chain of expressions multiple times. if (expression.expression.type !== 'FunctionCall') { register.statement(contract, expression); - if (expression.expression.name === 'assert' || expression.expression.name === 'require') { - register.assertOrRequire(contract, expression); + if (expression.expression.name === 'require') { + register.requireBranch(contract, expression); } parse[expression.expression.type] && parse[expression.expression.type](contract, expression.expression); diff --git a/lib/registrar.js b/lib/registrar.js index 0a3627da..cce5fbef 100644 --- a/lib/registrar.js +++ b/lib/registrar.js @@ -152,7 +152,7 @@ class Registrar { /** * Registers injections for branch measurements. This generic is consumed by - * the `assert/require` and `if` registration methods. + * the `require` and `if` registration methods. * @param {Object} contract instrumentation target * @param {Object} expression AST node */ @@ -187,17 +187,17 @@ class Registrar { }; /** - * Registers injections for assert/require statement measurements (branches) + * Registers injections for require statement measurements (branches) * @param {Object} contract instrumentation target * @param {Object} expression AST node */ - assertOrRequire(contract, expression) { + requireBranch(contract, expression) { this.addNewBranch(contract, expression); this._createInjectionPoint( contract, expression.range[0], { - type: 'injectAssertPre', + type: 'injectRequirePre', branchId: contract.branchId, } ); @@ -205,7 +205,7 @@ class Registrar { contract, expression.range[1] + 2, { - type: 'injectAssertPost', + type: 'injectRequirePost', branchId: contract.branchId, } ); diff --git a/test/units/assert.js b/test/units/assert.js index 4099bb05..593bdee6 100644 --- a/test/units/assert.js +++ b/test/units/assert.js @@ -16,7 +16,10 @@ describe('asserts and requires', () => { beforeEach(() => coverage = new Coverage()); after(async() => await api.finish()); - it('should cover assert statements as `if` statements when they pass', async function() { + // Assert was covered as a branch up to v0.7.11. But since those + // conditions are never meant to be fullfilled (and assert is really for smt) + // people disliked this... + it('should *not* cover assert statements as branches (pass)', async function() { const contract = await util.bootstrapCoverage('assert/Assert', api); coverage.addContract(contract.instrumented, util.filePath); await contract.instance.a(true); @@ -25,9 +28,7 @@ describe('asserts and requires', () => { assert.deepEqual(mapping[util.filePath].l, { 5: 1, }); - assert.deepEqual(mapping[util.filePath].b, { - 1: [1, 0], - }); + assert.deepEqual(mapping[util.filePath].b, {}); assert.deepEqual(mapping[util.filePath].s, { 1: 1, }); @@ -36,9 +37,9 @@ describe('asserts and requires', () => { }); }); - // NB: Truffle replays failing txs as .calls to obtain the revert reason from the return + // NB: truffle/contract replays failing txs as .calls to obtain the revert reason from the return // data. Hence the 2X measurements. - it('should cover assert statements as `if` statements when they fail', async function() { + it('should *not* cover assert statements as branches (fail)', async function() { const contract = await util.bootstrapCoverage('assert/Assert', api); coverage.addContract(contract.instrumented, util.filePath); @@ -48,9 +49,7 @@ describe('asserts and requires', () => { assert.deepEqual(mapping[util.filePath].l, { 5: 2, }); - assert.deepEqual(mapping[util.filePath].b, { - 1: [0, 2], - }); + assert.deepEqual(mapping[util.filePath].b, {}); assert.deepEqual(mapping[util.filePath].s, { 1: 2, });