search

gotwarlost / istanbul

Yet another JS code coverage tool that computes statement, line, function and branch coverage with module loader hooks to transparently add coverage when running tests. Supports all JS coverage use cases including unit tests, server side functional tests and browser tests. Built for scale.
Other
8.7k stars 786 forks source link

/* istanbul ignore next */ still damages/changes Function.toString() output #856

Open GerHobbelt opened 6 years ago

GerHobbelt commented 6 years ago

One can use /* istanbul ignore next */ to prevent istanbul / nyc from instrumenting the next JS statement/expression, which works well, except when used on a function which will be processed by .toString(f) via, for example, String(f) to obtain the function source code in string format. Then it turns out the not-instrumented function source code was rewritten after all via some whitespace minification process, resulting in, for example, unit tests checking such String(f) output, FAILING when you run them in a istanbul/nyc environment.

Example: https://github.com/GerHobbelt/jison/blob/master/packages/helpers-lib/tests/tests.js

These assertions all PASS when run in node/mocha:

  /* istanbul ignore next: test functions' code is injected and then crashes the test due to extra code coverage statements having been injected */ 
  it("printFunctionSourceCode (direct)", function () {
    function d(i) { /* mock for linters */ }

    assert.strictEqual(helpers.printFunctionSourceCode(function a(x) { return x; }), "function a(x) { return x; }");
    assert.strictEqual(helpers.printFunctionSourceCode(function (x)  { return x; }), "function (x)  { return x; }");
    assert.strictEqual(helpers.printFunctionSourceCode((x) => { return x; }), "(x) => { return x; }");
    assert.strictEqual(helpers.printFunctionSourceCode((x) => x), "(x) => x");
    assert.strictEqual(helpers.printFunctionSourceCode((x) => (d(1), d(2), x)), "(x) => (d(1), d(2), x)");
  });

  /* istanbul ignore next: test functions' code is injected and then crashes the test due to extra code coverage statements having been injected */ 
  it("printFunctionSourceCode (indirect)", function () {
    function d(i) { /* mock for linters */ }

    var f1 = function a(x) { return x; };
    var f2 = function (x)  { return x; };
    var f3 = (x) => { return x; };
    var f4 = (x) => x;
    var f5 = (x) => (d(1), d(2), x);

    assert.strictEqual(helpers.printFunctionSourceCode(f1), "function a(x) { return x; }");
    assert.strictEqual(helpers.printFunctionSourceCode(f2), "function (x)  { return x; }");
    assert.strictEqual(helpers.printFunctionSourceCode(f3), "(x) => { return x; }");
    assert.strictEqual(helpers.printFunctionSourceCode(f4), "(x) => x");
    assert.strictEqual(helpers.printFunctionSourceCode(f5), "(x) => (d(1), d(2), x)");
  });

while the same tests' code FAILS when executed in nyc+mocha with this error report:

  1) helpers API
       printFunctionSourceCode (direct):

      AssertionError: expected 'function a(x){return x;}' to equal 'function a(x) { return x; }'
      + expected - actual

      -function a(x){return x;}
      +function a(x) { return x; }

      at Context.<anonymous> (tests\tests.js:10:498)

  2) helpers API
       printFunctionSourceCode (indirect):

      AssertionError: expected 'function a(x){return x;}' to equal 'function a(x) { return x; }'
      + expected - actual

      -function a(x){return x;}
      +function a(x) { return x; }

      at Context.<anonymous> (tests\tests.js:10:1323)

which shows the whitespace minification of the functions-under-test by istanbul at work.

(Side note: here's the source code for the executed stringification helper printFunctionSourceCode():

function printFunctionSourceCode(f) {
    var src = String(f);             // <-- turn function source code into JS string
    chkBugger(src);
    return src;
}

AFAICT, this issue is closely related to #310, #674 with related discussion in https://github.com/GoogleChrome/puppeteer/issues/1054

GerHobbelt commented 6 years ago

The (TEMPORARY) fix in my own codebase uses run-time detection of istanbul, i.e. a function which checks if my code is executing in an istanbul/nyc environment.

Now the tests PASS in both normal mocha and nyc + mocha modes (compare to the tests in the OP):

  /* istanbul ignore next: test functions' code is injected and then crashes the test due to extra code coverage statements having been injected */ 
  it("printFunctionSourceCode (direct)", function () {
    function d(i) { /* mock for linters */ }

    if (!helpers.detectIstanbulGlobal()) {
      assert.strictEqual(helpers.printFunctionSourceCode(function a(x) { return x; }), "function a(x) { return x; }");
      assert.strictEqual(helpers.printFunctionSourceCode(function (x)  { return x; }), "function (x)  { return x; }");
      assert.strictEqual(helpers.printFunctionSourceCode((x) => { return x; }), "(x) => { return x; }");
      assert.strictEqual(helpers.printFunctionSourceCode((x) => x), "(x) => x");
      assert.strictEqual(helpers.printFunctionSourceCode((x) => (d(1), d(2), x)), "(x) => (d(1), d(2), x)");
      assert.strictEqual(helpers.printFunctionSourceCode(x => x + 7), "x => x + 7");
    } else {
      assert.strictEqual(helpers.printFunctionSourceCode(function a(x) { return x; }), "function a(x){return x;}");
      assert.strictEqual(helpers.printFunctionSourceCode(function (x)  { return x; }), "function (x){return x;}");
      assert.strictEqual(helpers.printFunctionSourceCode((x) => { return x; }), "x=>{return x;}");
      assert.strictEqual(helpers.printFunctionSourceCode((x) => x), "x=>x");
      assert.strictEqual(helpers.printFunctionSourceCode((x) => (d(1), d(2), x)), "x=>(d(1),d(2),x)");
      assert.strictEqual(helpers.printFunctionSourceCode(x => x + 7), "x=>x+7");
    }
  });

  /* istanbul ignore next: test functions' code is injected and then crashes the test due to extra code coverage statements having been injected */ 
  it("printFunctionSourceCode (indirect)", function () {
    function d(i) { /* mock for linters */ }

    var f1 = function a(x) { return x; };
    var f2 = function (x)  { return x; };
    var f3 = (x) => { return x; };
    var f4 = (x) => x;
    var f5 = (x) => (d(1), d(2), x);
    var f6 = x => x + 7;

    if (!helpers.detectIstanbulGlobal()) {
      assert.strictEqual(helpers.printFunctionSourceCode(f1), "function a(x) { return x; }");
      assert.strictEqual(helpers.printFunctionSourceCode(f2), "function (x)  { return x; }");
      assert.strictEqual(helpers.printFunctionSourceCode(f3), "(x) => { return x; }");
      assert.strictEqual(helpers.printFunctionSourceCode(f4), "(x) => x");
      assert.strictEqual(helpers.printFunctionSourceCode(f5), "(x) => (d(1), d(2), x)");
      assert.strictEqual(helpers.printFunctionSourceCode(f6), "x => x + 7");
    } else {
      assert.strictEqual(helpers.printFunctionSourceCode(f1), "function a(x){return x;}");
      assert.strictEqual(helpers.printFunctionSourceCode(f2), "function (x){return x;}");
      assert.strictEqual(helpers.printFunctionSourceCode(f3), "x=>{return x;}");
      assert.strictEqual(helpers.printFunctionSourceCode(f4), "x=>x");
      assert.strictEqual(helpers.printFunctionSourceCode(f5), "x=>(d(1),d(2),x)");
      assert.strictEqual(helpers.printFunctionSourceCode(f6), "x=>x+7");
    } 
  });

while this additional unit test for the istanbul detector helper has been added to make sure the detector itself isn't buggy:

  it("detectIstanbulGlobal", function () {
    if (!helpers.detectIstanbulGlobal()) {
      assert.strictEqual(helpers.detectIstanbulGlobal(), false);
    } else {
      var o = helpers.detectIstanbulGlobal();
      assert.ok(o);
      assert.equal(typeof o, 'object');
      var k = Object.keys(o);
      var kp = k.filter(function pass_paths(el) {
        return el.match(/[\\\/][^\\\/]+$/);
      });
      assert.ok(k.length > 0, "expected 1 or more keys in the istanbul global");
      assert.ok(kp.length > 0, "expected 1 or more paths as keys in the istanbul global");
      var kp = k.filter(function pass_istanbul_file_objects(idx) {
        var el = o[idx];
        return el && el.hash && el.statementMap && el.path;
      });
      assert.ok(kp.length > 0, "expected 1 or more istanbul file coverage objects in the istanbul global");
    }
  });

while the istanbul detector code looks like this:

// code to detect whether we're running under istanbul/nyc coverage environment...
function detectIstanbulGlobal() {
    const gcv = "__coverage__";
    const globalvar = new Function('return this')();
    var coverage = globalvar[gcv];
    return coverage || false;
}

export default detectIstanbulGlobal;

which was derived from the nyc instrument output of a test file.

Hope this helps someone out there while this issue is pending/open for istanbul...

tex0l commented 6 years ago

I confirm I have this bug, thanks for the workaround.