Incorrect branch coverage for async methods

[stefan-guggisberg]

• v12.22.5:
• macOS 11.5.2:

I've noticed some inexplicable uncovered branch reports after switching from nyc to c8. The uncovered branches were the closing braces of async methods. The issue can be reproduced with the following code:

class Test {
  constructor(msg) {
    this.msg = msg;
  }

  async say() {
    return this.msg;
  } // uncovered branch?
}

const main = async () => {
  const t = new Test('hello');
  console.log(await t.say());
};

main().catch(console.error);

Running c8:

npx c8 node src/test.js
hello
----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |     100 |       80 |     100 |     100 |                   
 test.js  |     100 |       80 |     100 |     100 | 8                
----------|---------|----------|---------|---------|-------------------

Line 8 is the closing } of the async test() method.

[coderaiser]

Same with me

c8 shows:

And on CI:

Here is simple reproducible repository https://github.com/coderaiser/changelog-io

[stefan-guggisberg]

and an even simpler test case to reproduce the issue:

const echo = async (msg) => {
  return msg;
};  // => uncovered branch??

const main = async () => {
  console.log(await echo('hello'));
};

main().catch(console.error);

npx c8 node src/test.js
hello
----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |     100 |       75 |     100 |     100 |                   
 test.js  |     100 |       75 |     100 |     100 | 3                 
----------|---------|----------|---------|---------|-------------------

so it's any async function causing the issue.

@bcoe I'd appreciate your feedback.

[bcoe]

@stefan-guggisberg this was likely a bug in v8 itself, which appears to have been fixed in newer versions of Node.js (I couldn't reproduce in v14/v16)

Unfortunately these updates to v8 can't always be back-ported to older Node.js versions (so I won't be able to fix in c8).

Can you potentially run coverage on a newer version of Node.js? while still testing on Node 12, i.e., run coverage on the latest and greatest Node.js, test on all versions you support.

[coderaiser]

@bcoe maybe it’s related to loaders, could you please take a look https://github.com/coderaiser/changelog-io/runs/3486781215?check_suite_focus=true.

Loader used to mock import call and change it from:

import fs from ‘fs’;

To:

const fs = __mockImport.get(‘fs’);

ESM doesn’t supports access to cache, so the only way to mock imports is using loaders.

[stefan-guggisberg]

Thanks @bcoe. The issue is reproducible on current Node v12 (12.22.6). I can confirm that it's not reproducible on Node v14 (14.17.6) and Node v16 (16.9.0).

[bcoe]

@stefan-guggisberg perhaps keep this issue open, so other folks understand this category of issue.