theintern / intern

A next-generation code testing stack for JavaScript.
https://theintern.io/
Other
4.36k stars 309 forks source link

Coverage is broken in Intern 4.5+ #1158

Closed jason0x43 closed 4 years ago

jason0x43 commented 4 years ago

In at least some scenarios, coverage in Intern 4.5+ is broken.

Example:

// src/app.ts
export function sayHi(name: string) {
  return `Hi, ${name}!`;
}
// tests/unit/app.ts
import { sayHi } from '../../src/app';

const { suite, test } = intern.getPlugin('interface.tdd');
const { assert } = intern.getPlugin('chai');

suite('app', () => {
  test('sayHi', () => {
    assert.equal(sayHi('Bob'), 'Hi, Bob!');
  });
});
// intern.json
{
  "coverage": ["src/*.js"],
  "suites": "tests/unit/app.js"
}

With Intern 4.8.5 on Node 10.15.3:

$ npx intern debug
DEBUG: Using default loader
DEBUG: Instrumenting /Users/jason/Temp/intern/intern4-test/src/app.js
DEBUG: Instrumenting /Users/jason/Temp/intern/intern4-test/src/app.js
(ノಠ益ಠ)ノ彡┻━┻
TypeError: Cannot read property 'f' of undefined
  at Object.<anonymous> @ src/app.js:1:0
  at Module._compile @ internal/modules/cjs/loader.js:701:30

This same example with Intern 4.3.6:

$ npx intern debug
DEBUG: Using default loader
DEBUG: Instrumenting /Users/jason/Temp/intern/intern4-test/src/app.js
DEBUG: Loaded suites: ["tests/unit/app.js"]
✓ node - app - sayHi (0.001s)

Total coverage
----------|----------|----------|----------|----------|-------------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------|----------|----------|----------|----------|-------------------|
All files |       70 |       40 |      100 |    77.78 |                   |
 app.js   |       70 |       40 |      100 |    77.78 |               6,7 |
----------|----------|----------|----------|----------|-------------------|
TOTAL: tested 1 platforms, 1 passed, 0 failed

Note that the app file is instrumented twice in 4.8.5. That's not good.

jason0x43 commented 4 years ago

4.8.0 exhibits the broken behavior.

jason0x43 commented 4 years ago

4.4.3 was the last version that doesn't exhibit the double instrumenting behavior.

jason0x43 commented 4 years ago

The istanbul dependencies were updated in Intern 4.5.0. In particular, istanbul-lib-coverage was updated from 1.1.0 to 2.0.1. Possibly that's the source of the issue.

jason0x43 commented 4 years ago

The problem is with istanbul-lib-hook. Downgrading to 1.2.2 works. Version > 1.2.2 will run both the "runInThisContext" and "require" hooks when importing files.

jason0x43 commented 4 years ago

The root problem is that, prior to the use of istanbul-lib-hook@2 in Intern 4.5, the runInThisContext hook was never executed (at least with Node 10+) because istanbul wasn't using the correct API for runInThisContext (https://github.com/istanbuljs/issues#89). The issue doesn't occur for Intern's self tests.

jason0x43 commented 4 years ago

Resolved in 1160f8e. Intern 5-pre did not have this issue.