Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coverage is broken in Intern 4.5+ #1158

Closed
jason0x43 opened this issue May 27, 2020 · 6 comments
Closed

Coverage is broken in Intern 4.5+ #1158

jason0x43 opened this issue May 27, 2020 · 6 comments
Assignees
Labels
bug Something that's not working as intended effort-medium This may take a couple of days priority-high Most important

Comments

@jason0x43
Copy link
Member

jason0x43 commented May 27, 2020

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 jason0x43 added bug Something that's not working as intended effort-medium This may take a couple of days priority-high Most important labels May 27, 2020
@jason0x43 jason0x43 self-assigned this May 27, 2020
@jason0x43
Copy link
Member Author

4.8.0 exhibits the broken behavior.

@jason0x43 jason0x43 changed the title Coverage is broken in Intern 4.8.5 Coverage is broken in Intern 4.8 May 27, 2020
@jason0x43
Copy link
Member Author

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

@jason0x43
Copy link
Member Author

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
Copy link
Member Author

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 jason0x43 changed the title Coverage is broken in Intern 4.8 Coverage is broken in Intern 4.5+ May 27, 2020
@jason0x43
Copy link
Member Author

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 added a commit that referenced this issue May 27, 2020
jason0x43 added a commit that referenced this issue May 28, 2020
jason0x43 added a commit that referenced this issue May 28, 2020
jason0x43 added a commit that referenced this issue May 28, 2020
@jason0x43
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that's not working as intended effort-medium This may take a couple of days priority-high Most important
Projects
None yet
Development

No branches or pull requests

1 participant