Add modifierWhitelist option (#597)

Author: cgewecke

README.md

@@ -85,6 +85,7 @@ module.exports = {
 | measureFunctionCoverage | *boolean* | `true` | Computes function coverage. [More...][34] |
 | measureModifierCoverage | *boolean* | `true` | Computes each modifier invocation as a code branch. [More...][34] |
 | matrixOutputPath | *String* | `./testMatrix.json` | Relative path to write test matrix JSON object to. [More...][39] |
+| modifierWhitelist | *String[]* | `[]` | List of modifier names (ex: "onlyOwner") to exclude from branch measurement. (Useful for modifiers which prepare something instead of acting as a gate.)) |
 | istanbulFolder | *String* | `./coverage` |  Folder location for Istanbul coverage reports. |
 | istanbulReporter | *Array* | `['html', 'lcov', 'text', 'json']` | [Istanbul coverage reporters][2]  |
 | mocha | *Object* | `{ }` | [Mocha options][3] to merge into existing mocha config. `grep` and `invert` are useful for skipping certain tests under coverage using tags in the test descriptions.|

lib/instrumenter.js

@@ -15,6 +15,7 @@ class Instrumenter {
   constructor(config={}){
     this.instrumentationData = {};
     this.injector = new Injector();
+    this.modifierWhitelist = config.modifierWhitelist || [];
     this.enabled = {
       statements: (config.measureStatementCoverage === false) ? false : true,
       functions: (config.measureFunctionCoverage === false) ? false: true,
@@ -62,7 +63,7 @@ class Instrumenter {
     const contract = {};
 
     this.injector.resetModifierMapping();
-    parse.configure(this.enabled);
+    parse.configure(this.enabled, this.modifierWhitelist);
 
     contract.source = contractSource;
     contract.instrumented = contractSource;

lib/parse.js

@@ -11,8 +11,9 @@ const FILE_SCOPED_ID = "fileScopedId";
 const parse = {};
 
 // Utilities
-parse.configure = function(_enabled){
+parse.configure = function(_enabled, _whitelist){
   register.enabled = Object.assign(register.enabled, _enabled);
+  register.modifierWhitelist = _whitelist;
 }
 
 // Nodes

lib/registrar.js

@@ -19,6 +19,8 @@ class Registrar {
       branches: true,
       lines: true
     }
+
+    this.modifierWhitelist = [];
   }
 
   /**
@@ -129,7 +131,13 @@ class Registrar {
         }
 
         // Add modifier branch coverage
-        if (!this.enabled.modifiers || expression.isConstructor) continue;
+        if (
+          !this.enabled.modifiers   ||
+          expression.isConstructor  ||
+          this.modifierWhitelist.includes(modifier.name)
+        ) {
+          continue;
+        }
 
         this.addNewBranch(contract, modifier);
         this._createInjectionPoint(

lib/validator.js

@@ -42,6 +42,11 @@ const configSchema = {
       type: "array",
       items: {type: "string"}
     },
+
+    modifierWhitelist: {
+      type: "array",
+      items: {type: "string"}
+    }
   },
 };
 

test/units/modifiers.js

@@ -13,7 +13,10 @@ describe('modifiers', () => {
     api = new Api({silent: true});
     await api.ganache(client);
   })
-  beforeEach(() => coverage = new Coverage());
+  beforeEach(() => {
+    api.config = {};
+    coverage = new Coverage()
+  });
   after(async() => await api.finish());
 
   async function setupAndRun(solidityFile){
@@ -97,6 +100,25 @@ describe('modifiers', () => {
     });
   });
 
+  // Same test as above - should have 2 fewer branches
+  it('should exclude whitelisted modifiers', async function() {
+    api.config.modifierWhitelist = ['mmm', 'nnn'];
+    const mapping = await setupAndRun('modifiers/multiple-mods-same-fn');
+
+    assert.deepEqual(mapping[util.filePath].l, {
+      5: 1, 6: 1, 10: 1, 11: 1, 15: 1
+    });
+    assert.deepEqual(mapping[util.filePath].b, {
+      1: [1, 0], 2: [1, 0]
+    });
+    assert.deepEqual(mapping[util.filePath].s, {
+      1: 1, 2: 1, 3: 1
+    });
+    assert.deepEqual(mapping[util.filePath].f, {
+      1: 1, 2: 1, 3: 1
+    });
+  });
+
   it('should cover multiple functions which use the same modifier', async function() {
     const contract = await util.bootstrapCoverage('modifiers/multiple-fns-same-mod', api);
     coverage.addContract(contract.instrumented, util.filePath);

test/units/validator.js

@@ -119,6 +119,7 @@ describe('config validation', () => {
     const options =  [
       "skipFiles",
       "istanbulReporter",
+      "modifierWhitelist"
     ]
 
     options.forEach(name => {