Incorrect branch coverage when loaders used

[coderaiser]

c8 shows uncovered lines, but the whole file is covered.

Version: output of node -v 16.9.0 Platform: output of uname -a Darwin

• Version: latest
• Platform: mac os

Repository https://github.com/coderaiser/c8-reproduce

When I'm using loaders to mock imports the coverage I see is broken.

Code:

import {
    readFile,
} from 'fs/promises';

import {
    execSync,
} from 'child_process';

export default (a, b, c) => {
    if (a)
        return readFile();

      if (c)
          return execSync();

      return 'd';
};

Tests:

import {createMockImport} from 'mock-import';
import {
    test,
    stub,
} from 'supertape';

const {mockImport, reImport, stopAll} = createMockImport(import.meta.url);

test('changelog: a', async (t) => {
    mockImport('fs/promises', {
        readFile: stub().returns('a'),
    });
    const fn = await reImport('./changelog.js');
    stopAll();

    t.equal(fn.default(1), 'a');
});

test('changelog: c', async (t) => {
    mockImport('child_process', {
        execSync: stub().returns('c'),
    });

    const fn = await reImport('./changelog.js');
    stopAll();

    t.equal(fn.default(0, 0, 1), 'c');
});

test('changelog: d', async (t) => {
    const fn = await import('./changelog.js?count=4');

    t.equal(fn.default(0, 0, 0, 1), 'd');
});

What mock-import does is converts source to:

const {
    readFile: readFile
} = global.__mockImportCache.get('fs/promises');

import {
    execSync,
} from 'child_process';

export default (a, b, c) => {
    if (a)
        return readFile();

      if (c)
          return execSync();

      return 'd';
};

And imports it as ./changelog.js?count=1 on first test, then on second test:

import {
    readFile
} from 'fs/promises'

const {
    execSync,
} = global.__mockImportCache.get('fs/promises');

export default (a, b, c) => {
    if (a)
        return readFile();

      if (c)
          return execSync();

      return 'd';
};

File imported as ./changelog.js?count=2, and then on third assertion code isn't changed.

[bcoe]

@coderaiser thanks for the bug report. I'm not super familiar with the loader API, so not sure where this bug might be cropping up -- it could be as simple as coverage reports not being output by Node.js in one edge case (which would be relatively easy to fix).

If the problem is missing lines of coverage in v8 itself, this tends to be more difficult to fix.

If you beat me to the punch, and are excited to dig into this, where I would start looking would be in the raw v8 output:

cat coverage/tmp/coverage-661
coverage-66107-1631201558396-0.json  coverage-66110-1631201558266-0.json  

These files contain the raw output from v8 (which is written to disk by Node.js on exit):

{"result":[{"scriptId":"6","url":"internal/per_context/primordials.js","functions":[{"functionName":"copyProps","ranges":[{"startOffset":824,"endOffset":1077,"count":4},{"startOffset":898,"endOffset":1075,"count":13},{"startOffset":954,"endOffset":1071,"count":5}],"isBlockCoverage":true},{"functionName":"SafeIterator","ranges":[{"startOffset":2911,"endOffset":2982,"count":3}],"isBlockCoverage":true},{"functionName":"next","ranges":[{"startOffset":2987,"endOffset":3036,"count":374}],"isBlockCoverage":true},{"functionName":"makeSafe","ranges":[{"startOffset":3246,"endOffset":4378,"count":2},{"startOffset":3323,"endOffset":4170,"count":0}],"isBlockCoverage":true},{"functionName":"desc.value","ranges":[{"startOffset":4015,"endOffset":4082,"count":3}],"isBlockCoverage":true},{"functionName":"SafeMap","ranges":[{"startOffset":4717,"endOffset":4745,"count":16}],"isBlockCoverage":true},{"functionName":"SafeWeakMap","ranges":[{"startOffset":4888,"endOffset":4916,"count":7}]

☝️ my guess is that we'll see output that's slightly off startOffets/endOffset values for code using the loaders.

[coderaiser]

Just landed ability to generate sourcemap according to file url received from loader. Since mock-import does some changes, sourcemap should help to identify correct locations of code parts.

Generated file looks this way:

const hello = 'world';
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImhlbGxvLmpzIl0sIm5    hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsRUFBRSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyx    DQUFDIiwiZmlsZSI6ImhlbGxvLm1hcCIsInNvdXJjZXNDb250ZW50IjpbImNvbnN0IGhlbGxvID0gJ3dvcmxkJzsiXX0=

And it works good in the browser. The question is: does c8 supports instrumented code? I see that there is an option --exclude-after-remap but I don't see any changes in coverage information.

Could you please help me to get source map working?

[coderaiser]

Looks like istrumenting with babel-plugin-istanbul helped to solve the problem on a half! No it works full coverage all the time 😄 .

Happy holidays 🎉 !

[bcoe]

@coderaiser If I understand correctly, you're using babel-plugin-istanbul rather than c8, sounds like it's working for you and I can close this?

[coderaiser]

@bcoe Unfortunately it shows full coverage every time, so it’s useless. Could you please tell me is it possible to make c8 read source maps?

Source file is changed inside of loader, and it has different name(with ?mock-import=n) to have ability to import file again with mocked import. It works very good, but sometimes (like in this example) shoes bad lines and columns positions, and shows that some code is uncovered, but it is covered.

[coderaiser]

Looks like it can be related to _normalizeMapCache and _normalizeProcessCov in /lib/report.js. Both uses fileURLToPath which is cut down ?mock-import-count=1 suffixes.

But they are needed to import file again and transform it's content with mocked modules. Is it possible to cut them after we merged reports related to all files with prefix?

[coderaiser]

Looks like this is 1:many case from v8-to-instanbul function named load:

• ✅ 1 source code;
• ✅ many source maps;

We should merge all changes that we find using different source maps (for each test when import is mocked).

[bcoe]

Hey @coderaiser, can you figure out a minimal failing test case? that demonstrates how truncating the query string causes this bug?

I think we could probably figure out an approach to keep the suffix longer, until the source map lookup occurs, if that's the root of the problem (I think I'm not yet fully understanding, what do the entries for sourcemaps look like In the cache, they include the query string?)

[coderaiser]

Hey @bcoe, OK, right now I have 3 tests.

they include the query string?

Sourcemap doesn't includes query string, it includes path of a file on disk.

When readFile mocked:

const {
    readFile: readFile
} = global.__mockImportCache.get('fs/promises');

import {
    execSync,
} from 'child_process';

export default (a, b, c) => {
    if (a)
        return readFile();

      if (c)
          return execSync();

      return 'd';
};

//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImZpbGU6Ly8vVXNlcnMvY29kZXJhaXNlci9jOC1yZXByb2R1Y2UvbGliL2NoYW5nZWxvZy5qcyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiOztpQ0FFTyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQzs7QUFFbkIsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUU7SUFDSCxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUM7QUFDWixFQUFFLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUM7O0FBRXRCLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxFQUFFLENBQUMsRUFBRSxDQUFDLEVBQUU7SUFDeEIsQ0FBQyxFQUFFLENBQUMsQ0FBQztRQUNELENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUM7O01BRW5CLENBQUMsRUFBRSxDQUFDLENBQUM7VUFDRCxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDOztNQUVyQixDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQztBQUNoQixDQUFDIiwiZmlsZSI6ImZpbGU6Ly8vVXNlcnMvY29kZXJhaXNlci9jOC1yZXByb2R1Y2UvbGliL2NoYW5nZWxvZy5qcy5tYXAiLCJzb3VyY2VzQ29udGVudCI6WyJpbXBvcnQge1xuICAgIHJlYWRGaWxlLFxufSBmcm9tICdmcy9wcm9taXNlcyc7XG5cbmltcG9ydCB7XG4gICAgZXhlY1N5bmMsXG59IGZyb20gJ2NoaWxkX3Byb2Nlc3MnO1xuXG5leHBvcnQgZGVmYXVsdCAoYSwgYiwgYykgPT4ge1xuICAgIGlmIChhKVxuICAgICAgICByZXR1cm4gcmVhZEZpbGUoKTtcbiAgICAgXG4gICAgICBpZiAoYylcbiAgICAgICAgICByZXR1cm4gZXhlY1N5bmMoKTtcbiAgICAgIFxuICAgICAgcmV0dXJuICdkJztcbn07XG5cbiJdfQ==

Here is version Base64 encoded:

{
  "version": 3,
  "sources": [
    "file:///Users/coderaiser/c8-reproduce/lib/changelog.js"
  ],
  "names": [],
  "mappings": ";;iCAEO,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;AAEnB,CAAC,CAAC,CAAC,CAAC,CAAC,EAAE;IACH,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;AACZ,EAAE,CAAC,CAAC,CAAC,EAAE,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;AAEtB,CAAC,CAAC,CAAC,CAAC,CAAC,EAAE,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,EAAE,CAAC,CAAC,EAAE,CAAC,EAAE,CAAC,EAAE,CAAC,EAAE;IACxB,CAAC,EAAE,CAAC,CAAC;QACD,CAAC,CAAC,CAAC,CAAC,CAAC,EAAE,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;MAEnB,CAAC,EAAE,CAAC,CAAC;UACD,CAAC,CAAC,CAAC,CAAC,CAAC,EAAE,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC,CAAC;;MAErB,CAAC,CAAC,CAAC,CAAC,CAAC,EAAE,CAAC,CAAC,CAAC;AAChB,CAAC",
  "file": "file:///Users/coderaiser/c8-reproduce/lib/changelog.js.map",
  "sourcesContent": [
    "import {\\n    readFile,\\n} from 'fs/promises';\\n\\nimport {\\n    execSync,\\n} from 'child_process';\\n\\nexport default (a, b, c) => {\\n    if (a)\\n        return readFile();\\n     \\n      if (c)\\n          return execSync();\\n      \\n      return 'd';\\n};\\n\\n"
  ]
}

When execSync mocked:

import {
    readFile,
} from 'fs/promises';

const {
    execSync: execSync
} = global.__mockImportCache.get('child_process');

export default (a, b, c) => {
    if (a)
        return readFile();

      if (c)
          return execSync();

      return 'd';
};

//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImZpbGU6Ly8vVXNlcnMvY29kZXJhaXNlci9jOC1yZXByb2R1Y2UvbGliL2NoYW5nZWxvZy5qcyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRTtJQUNILENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQztBQUNaLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDOzs7O2lDQUliLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQzs7QUFFckIsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsRUFBRSxDQUFDLEVBQUUsQ0FBQyxFQUFFLENBQUMsRUFBRTtJQUN4QixDQUFDLEVBQUUsQ0FBQyxDQUFDO1FBQ0QsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQzs7TUFFbkIsQ0FBQyxFQUFFLENBQUMsQ0FBQztVQUNELENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUM7O01BRXJCLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDO0FBQ2hCLENBQUMiLCJmaWxlIjoiZmlsZTovLy9Vc2Vycy9jb2RlcmFpc2VyL2M4LXJlcHJvZHVjZS9saWIvY2hhbmdlbG9nLmpzLm1hcCIsInNvdXJjZXNDb250ZW50IjpbImltcG9ydCB7XG4gICAgcmVhZEZpbGUsXG59IGZyb20gJ2ZzL3Byb21pc2VzJztcblxuaW1wb3J0IHtcbiAgICBleGVjU3luYyxcbn0gZnJvbSAnY2hpbGRfcHJvY2Vzcyc7XG5cbmV4cG9ydCBkZWZhdWx0IChhLCBiLCBjKSA9PiB7XG4gICAgaWYgKGEpXG4gICAgICAgIHJldHVybiByZWFkRmlsZSgpO1xuICAgICBcbiAgICAgIGlmIChjKVxuICAgICAgICAgIHJldHVybiBleGVjU3luYygpO1xuICAgICAgXG4gICAgICByZXR1cm4gJ2QnO1xufTtcblxuIl19

When nothing mocked:

import {
    readFile,
} from 'fs/promises';

import {
    execSync,
} from 'child_process';

export default (a, b, c) => {
    if (a)
        return readFile();

      if (c)
          return execSync();

      return 'd';
};

//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImZpbGU6Ly8vVXNlcnMvY29kZXJhaXNlci9jOC1yZXByb2R1Y2UvbGliL2NoYW5nZWxvZy5qcyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRTtJQUNILENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQztBQUNaLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDOztBQUVwQixDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRTtJQUNILENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQztBQUNaLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQzs7QUFFdEIsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsRUFBRSxDQUFDLEVBQUUsQ0FBQyxFQUFFLENBQUMsRUFBRTtJQUN4QixDQUFDLEVBQUUsQ0FBQyxDQUFDO1FBQ0QsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQzs7TUFFbkIsQ0FBQyxFQUFFLENBQUMsQ0FBQztVQUNELENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUM7O01BRXJCLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDO0FBQ2hCLENBQUMiLCJmaWxlIjoiZmlsZTovLy9Vc2Vycy9jb2RlcmFpc2VyL2M4LXJlcHJvZHVjZS9saWIvY2hhbmdlbG9nLmpzLm1hcCIsInNvdXJjZXNDb250ZW50IjpbImltcG9ydCB7XG4gICAgcmVhZEZpbGUsXG59IGZyb20gJ2ZzL3Byb21pc2VzJztcblxuaW1wb3J0IHtcbiAgICBleGVjU3luYyxcbn0gZnJvbSAnY2hpbGRfcHJvY2Vzcyc7XG5cbmV4cG9ydCBkZWZhdWx0IChhLCBiLCBjKSA9PiB7XG4gICAgaWYgKGEpXG4gICAgICAgIHJldHVybiByZWFkRmlsZSgpO1xuICAgICBcbiAgICAgIGlmIChjKVxuICAgICAgICAgIHJldHVybiBleGVjU3luYygpO1xuICAgICAgXG4gICAgICByZXR1cm4gJ2QnO1xufTtcblxuIl19

Hey @coderaiser, can you figure out a minimal failing test case? that demonstrates how truncating the query string causes this bug?

Here is repository and test, to get things working run:

git clone https://github.com/coderaiser/c8-reproduce
cd c8-reproduce
npm i
npm run coverage

You can use npm run debug for debugging.

And you will see that coverage information is not full:

 changelog.js |     100 |       20 |     100 |     100 | 11-14

Here is PR with partial support of sourcemaps, it keeps file URLS as they are, but the problem still exist in https://github.com/bcoe/c8/blob/v7.11.0/lib/report.js#L116, as it works with v8 coverage offsets which are different, I saw that v8-to-instanbul parses this differences according to sourcemaps but I'm not quit understand how it works. It returns Infinity for line and column in some cases.

https://github.com/bcoe/c8/pull/345.

[coderaiser]

Here is offsets I receive in v8ProcessCov:

{
    "result": [
        {
            "scriptId": "0",
            "url": "file:///Users/coderaiser/c8-reproduce/lib/changelog.js?mock-import-count=1",
            "functions": [
                {
                    "functionName": "",
                    "ranges": [
                        {
                            "startOffset": 0,
                            "endOffset": 1824,
                            "count": 1
                        }
                    ],
                    "isBlockCoverage": true
                },
                {
                    "functionName": "default",
                    "ranges": [
                        {
                            "startOffset": 144,
                            "endOffset": 271,
                            "count": 1
                        },
                        {
                            "startOffset": 196,
                            "endOffset": 270,
                            "count": 0
                        }
                    ],
                    "isBlockCoverage": true
                }
            ]
        },
        {
            "scriptId": "1",
            "url": "file:///Users/coderaiser/c8-reproduce/lib/changelog.js?mock-import-count=2",
            "functions": [
                {
                    "functionName": "",
                    "ranges": [
                        {
                            "startOffset": 0,
                            "endOffset": 1820,
                            "count": 1
                        }
                    ],
                    "isBlockCoverage": true
                },
                {
                    "functionName": "default",
                    "ranges": [
                        {
                            "startOffset": 144,
                            "endOffset": 271,
                            "count": 1
                        },
                        {
                            "startOffset": 178,
                            "endOffset": 196,
                            "count": 0
                        },
                        {
                            "startOffset": 244,
                            "endOffset": 270,
                            "count": 0
                        }
                    ],
                    "isBlockCoverage": true
                }
            ]
        },
        {
            "scriptId": "2",
            "url": "file:///Users/coderaiser/c8-reproduce/lib/changelog.js?mock-import-count=3",
            "functions": [
                {
                    "functionName": "",
                    "ranges": [
                        {
                            "startOffset": 0,
                            "endOffset": 1929,
                            "count": 1
                        }
                    ],
                    "isBlockCoverage": true
                },
                {
                    "functionName": "default",
                    "ranges": [
                        {
                            "startOffset": 109,
                            "endOffset": 236,
                            "count": 1
                        },
                        {
                            "startOffset": 143,
                            "endOffset": 161,
                            "count": 0
                        },
                        {
                            "startOffset": 191,
                            "endOffset": 209,
                            "count": 0
                        }
                    ],
                    "isBlockCoverage": true
                }
            ]
        }]
}

And they are passed to applyCoverage from v8-to-instanbul.

[bcoe]

I dug into the problem a bit more, the root of the problem is essentially that the three changelog.js files are being treated as the same file, due to the similar name, but in practice their in memory representation is three different files (it seems like their transpiled wrappers differ, and the source map at the end of the file also differs, leading to discrepancies in the byte length).

Here's an example of why this would cause trouble merging:

changelog.js?mock-import-3:

        {
          "functionName": "default",
          "ranges": [
            {
              "startOffset": 109,
              "endOffset": 236,
              "count": 1
            }
        }

vs., changelog.js?mock-import-2:

        {
          "functionName": "default",
          "ranges": [
            {
              "startOffset": 144,
              "endOffset": 271,
              "count": 1
            }
}

changelog.js?mock-import-1:

        {
          "functionName": "default",
          "ranges": [
            {
              "startOffset": 144,
              "endOffset": 271,
              "count": 1
            }
    }

Note that in mock-import-1 an mock-import-2, the default method is the range 144 - 271, but in mock-import-3 it falls in the position 109 - 236, this breaks the logic that's applied to merge the v8 format output together.

To address this problem, I think you would need to:

1. treat these files as distinct files, rather than applying the merging logic.
2. run each individual file through v8-to-istanbul with its own SourceMap.
3. extend v8-to-istanbul with logic to merge reports post-hoc, rather than merging the v8 format output.

There's a question of whether the Istanbul format output will merge cleanly, I haven't tried this before.

[bcoe]

I hacked together an approach that merges three coverage reports at the end, rather than merging the v8 output at the start, it kind of works:

However, it has trouble merging branches, this isn't unexpected because SourceMaps provide sparse data, so trying to combine two source maps can be like comparing apples and oranges.

A better way to fix your problem, I think, would be to figure out where the 30 byte discrepancy is happening in loader-3, vs., loader-2 and loader-1. If you can get these byte ranges to correlate, c8 will start working -- my guess is a header is being injected by 1/2 but is not being injected for 3 ... perhaps a different function wrapper?

[coderaiser]

run each individual file through v8-to-istanbul with its own SourceMap.

I don't quite understand, right now each files is converted: https://github.com/bcoe/c8/blob/eaf8456d88d5a407bd79641b7e71b9cb50ad4434/lib/report.js#L93-L102

it goes to v8toIstanbul and then get's to load where we get originalRawSource which is accessible from source map, and it is the same for all three sourcemaps:

import {
    readFile,
} from 'fs/promises';

import {
    execSync,
} from 'child_process';

export default (a, b, c) => {
    if (a)
        return readFile();

      if (c)
          return execSync();

      return 'd';
};

Then for some reason this.path passed to new CovSource in sources.js, but it never used 🤷‍♂️

Then sourceTranspiled made from rawSource:

.......
......................
................................................

........
.............
.......................

.............................
..........
..........................
.....
............
............................
......
.................
..

............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

And than saves value of this.sourceTranspiled:

{
    "lines": [
        {
            "line": 1,
            "startCol": 0,
            "endCol": 7,
            "count": 1,
            "ignore": false
        },
        {
            "line": 2,
            "startCol": 8,
            "endCol": 30,
            "count": 1,
            "ignore": false
        },
        {
            "line": 3,
            "startCol": 31,
            "endCol": 79,
            "count": 1,
            "ignore": false
        },
        {
            "line": 4,
            "startCol": 80,
            "endCol": 80,
            "count": 1,
            "ignore": false
        },
        {
            "line": 5,
            "startCol": 81,
            "endCol": 89,
            "count": 1,
            "ignore": false
        },
        {
            "line": 6,
            "startCol": 90,
            "endCol": 103,
            "count": 1,
            "ignore": false
        },
        {
            "line": 7,
            "startCol": 104,
            "endCol": 127,
            "count": 1,
            "ignore": false
        },
        {
            "line": 8,
            "startCol": 128,
            "endCol": 128,
            "count": 1,
            "ignore": false
        },
        {
            "line": 9,
            "startCol": 129,
            "endCol": 158,
            "count": 1,
            "ignore": false
        },
        {
            "line": 10,
            "startCol": 159,
            "endCol": 169,
            "count": 1,
            "ignore": false
        },
        {
            "line": 11,
            "startCol": 170,
            "endCol": 196,
            "count": 1,
            "ignore": false
        },
        {
            "line": 12,
            "startCol": 197,
            "endCol": 202,
            "count": 1,
            "ignore": false
        },
        {
            "line": 13,
            "startCol": 203,
            "endCol": 215,
            "count": 1,
            "ignore": false
        },
        {
            "line": 14,
            "startCol": 216,
            "endCol": 244,
            "count": 1,
            "ignore": false
        },
        {
            "line": 15,
            "startCol": 245,
            "endCol": 251,
            "count": 1,
            "ignore": false
        },
        {
            "line": 16,
            "startCol": 252,
            "endCol": 269,
            "count": 1,
            "ignore": false
        },
        {
            "line": 17,
            "startCol": 270,
            "endCol": 272,
            "count": 1,
            "ignore": false
        },
        {
            "line": 18,
            "startCol": 273,
            "endCol": 273,
            "count": 1,
            "ignore": false
        },
        {
            "line": 19,
            "startCol": 274,
            "endCol": 274,
            "count": 1,
            "ignore": false
        },
        {
            "line": 20,
            "startCol": 275,
            "endCol": 1823,
            "count": 1,
            "ignore": false
        }
    ],
    "eof": 1823,
    "shebangLength": 0,
    "wrapperLength": 0
}

in practice their in memory representation is three different files (it seems like their transpiled wrappers differ, and the source map at the end of the file also differs, leading to discrepancies in the byte length).

That's right, this is 3 different files:

const {
    readFile: readFile
} = global.__mockImportCache.get('fs/promises');

import {
    execSync,
} from 'child_process';

export default (a, b, c) => {
    if (a)
        return readFile();

      if (c)
          return execSync();

      return 'd';
};

//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImZpbGU6Ly8vVXNlcnMvY29kZXJhaXNlci9jOC1yZXByb2R1Y2UvbGliL2NoYW5nZWxvZy5qcyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiOztpQ0FFTyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQzs7QUFFbkIsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUU7SUFDSCxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUM7QUFDWixFQUFFLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUM7O0FBRXRCLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxFQUFFLENBQUMsRUFBRSxDQUFDLEVBQUU7SUFDeEIsQ0FBQyxFQUFFLENBQUMsQ0FBQztRQUNELENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUM7O01BRW5CLENBQUMsRUFBRSxDQUFDLENBQUM7VUFDRCxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDOztNQUVyQixDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQztBQUNoQixDQUFDIiwiZmlsZSI6ImZpbGU6Ly8vVXNlcnMvY29kZXJhaXNlci9jOC1yZXByb2R1Y2UvbGliL2NoYW5nZWxvZy5qcy5tYXAiLCJzb3VyY2VzQ29udGVudCI6WyJpbXBvcnQge1xuICAgIHJlYWRGaWxlLFxufSBmcm9tICdmcy9wcm9taXNlcyc7XG5cbmltcG9ydCB7XG4gICAgZXhlY1N5bmMsXG59IGZyb20gJ2NoaWxkX3Byb2Nlc3MnO1xuXG5leHBvcnQgZGVmYXVsdCAoYSwgYiwgYykgPT4ge1xuICAgIGlmIChhKVxuICAgICAgICByZXR1cm4gcmVhZEZpbGUoKTtcbiAgICAgXG4gICAgICBpZiAoYylcbiAgICAgICAgICByZXR1cm4gZXhlY1N5bmMoKTtcbiAgICAgIFxuICAgICAgcmV0dXJuICdkJztcbn07XG5cbiJdfQ==

Second:

import {
    readFile,
} from 'fs/promises';

const {
    execSync: execSync
} = global.__mockImportCache.get('child_process');

export default (a, b, c) => {
    if (a)
        return readFile();

      if (c)
          return execSync();

      return 'd';
};

//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImZpbGU6Ly8vVXNlcnMvY29kZXJhaXNlci9jOC1yZXByb2R1Y2UvbGliL2NoYW5nZWxvZy5qcyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRTtJQUNILENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQztBQUNaLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDOzs7O2lDQUliLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQzs7QUFFckIsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsRUFBRSxDQUFDLEVBQUUsQ0FBQyxFQUFFLENBQUMsRUFBRTtJQUN4QixDQUFDLEVBQUUsQ0FBQyxDQUFDO1FBQ0QsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQzs7TUFFbkIsQ0FBQyxFQUFFLENBQUMsQ0FBQztVQUNELENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUM7O01BRXJCLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDO0FBQ2hCLENBQUMiLCJmaWxlIjoiZmlsZTovLy9Vc2Vycy9jb2RlcmFpc2VyL2M4LXJlcHJvZHVjZS9saWIvY2hhbmdlbG9nLmpzLm1hcCIsInNvdXJjZXNDb250ZW50IjpbImltcG9ydCB7XG4gICAgcmVhZEZpbGUsXG59IGZyb20gJ2ZzL3Byb21pc2VzJztcblxuaW1wb3J0IHtcbiAgICBleGVjU3luYyxcbn0gZnJvbSAnY2hpbGRfcHJvY2Vzcyc7XG5cbmV4cG9ydCBkZWZhdWx0IChhLCBiLCBjKSA9PiB7XG4gICAgaWYgKGEpXG4gICAgICAgIHJldHVybiByZWFkRmlsZSgpO1xuICAgICBcbiAgICAgIGlmIChjKVxuICAgICAgICAgIHJldHVybiBleGVjU3luYygpO1xuICAgICAgXG4gICAgICByZXR1cm4gJ2QnO1xufTtcblxuIl19

And third:

import {
    readFile,
} from 'fs/promises';

import {
    execSync,
} from 'child_process';

export default (a, b, c) => {
    if (a)
        return readFile();

      if (c)
          return execSync();

      return 'd';
};

//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImZpbGU6Ly8vVXNlcnMvY29kZXJhaXNlci9jOC1yZXByb2R1Y2UvbGliL2NoYW5nZWxvZy5qcyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRTtJQUNILENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQztBQUNaLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDOztBQUVwQixDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRTtJQUNILENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQztBQUNaLEVBQUUsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQzs7QUFFdEIsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsRUFBRSxDQUFDLENBQUMsRUFBRSxDQUFDLEVBQUUsQ0FBQyxFQUFFLENBQUMsRUFBRTtJQUN4QixDQUFDLEVBQUUsQ0FBQyxDQUFDO1FBQ0QsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLEVBQUUsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQzs7TUFFbkIsQ0FBQyxFQUFFLENBQUMsQ0FBQztVQUNELENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxDQUFDLENBQUM7O01BRXJCLENBQUMsQ0FBQyxDQUFDLENBQUMsQ0FBQyxFQUFFLENBQUMsQ0FBQyxDQUFDO0FBQ2hCLENBQUMiLCJmaWxlIjoiZmlsZTovLy9Vc2Vycy9jb2RlcmFpc2VyL2M4LXJlcHJvZHVjZS9saWIvY2hhbmdlbG9nLmpzLm1hcCIsInNvdXJjZXNDb250ZW50IjpbImltcG9ydCB7XG4gICAgcmVhZEZpbGUsXG59IGZyb20gJ2ZzL3Byb21pc2VzJztcblxuaW1wb3J0IHtcbiAgICBleGVjU3luYyxcbn0gZnJvbSAnY2hpbGRfcHJvY2Vzcyc7XG5cbmV4cG9ydCBkZWZhdWx0IChhLCBiLCBjKSA9PiB7XG4gICAgaWYgKGEpXG4gICAgICAgIHJldHVybiByZWFkRmlsZSgpO1xuICAgICBcbiAgICAgIGlmIChjKVxuICAgICAgICAgIHJldHVybiBleGVjU3luYygpO1xuICAgICAgXG4gICAgICByZXR1cm4gJ2QnO1xufTtcblxuIl19

my guess is a header is being injected by 1/2 but is not being injected for 3 ... perhaps a different function wrapper?

3-rd file is the same as original, but it still has sourcemap added by recast. If I don't generate sourcemap for third file result is the same:

changelog.js | 100 | 42.85 | 100 | 100 | 11-14

I hacked together an approach that merges three coverage reports at the end, rather than merging the v8 output at the start, it kind of works

This is amazing! Would be great if you add this to c8 :).

[bcoe]

This is amazing! Would be great if you add this to c8 :)

Yes I'd be open to adding this functionality, the only issue is it does draw attention to the fact that merging multiple istanbul reports is a bit buggy.

The function and line coverage should be pretty accurate, I think, but as demonstrated by the yellow blocks in the report I shared, branch coverage is a bit off.

All I did was stop dropping the ? suffix:

diff --git a/lib/report.js b/lib/report.js
index d3c8806..41edabf 100644
--- a/lib/report.js
+++ b/lib/report.js
@@ -117,6 +117,7 @@ class Report {
           map.merge(converter.toIstanbul())
         }
       } catch (err) {
+        console.info(err)
         debuglog(`file: ${v8ScriptCov.url} error: ${err.stack}`)
       }
     }
@@ -143,7 +144,12 @@ class Report {
    */
   _getSourceMap (v8ScriptCov) {
     const sources = {}
-    const sourceMapAndLineLengths = this.sourceMapCache[pathToFileURL(v8ScriptCov.url).href]
+    let suffix = '';
+    const match = v8ScriptCov.url.match(/(?<query>\?.*)$/)
+    if (match) {
+      suffix = match.groups.query;
+    }
+    const sourceMapAndLineLengths = this.sourceMapCache['file://' + v8ScriptCov.url]
     if (sourceMapAndLineLengths) {
       // See: https://github.com/nodejs/node/pull/34305
       if (!sourceMapAndLineLengths.data) return
@@ -279,15 +285,22 @@ class Report {
       }
       if (/^file:\/\//.test(v8ScriptCov.url)) {
         try {
-          v8ScriptCov.url = fileURLToPath(v8ScriptCov.url)
-          fileIndex.add(v8ScriptCov.url)
+          let suffix = '';
+          const match = v8ScriptCov.url.match(/(?<query>\?.*)$/)
+          if (match) {
+            suffix = match.groups.query;
+          }
+          const normalized = fileURLToPath(v8ScriptCov.url)
+          v8ScriptCov.url = normalized + suffix;
+          fileIndex.add(normalized)
         } catch (err) {
           debuglog(`${err.stack}`)
           continue
         }
       }
       if ((!this.omitRelative || isAbsolute(v8ScriptCov.url))) {
-        if (this.excludeAfterRemap || this.exclude.shouldInstrument(v8ScriptCov.url)) {
+        const url = v8ScriptCov.url.split('?')[0]
+        if (this.excludeAfterRemap || this.exclude.shouldInstrument(url)) {
           result.push(v8ScriptCov)
         }
       }
@@ -307,7 +320,12 @@ class Report {
   _normalizeSourceMapCache (v8SourceMapCache) {
     const cache = {}
     for (const fileURL of Object.keys(v8SourceMapCache)) {
-      cache[pathToFileURL(fileURLToPath(fileURL)).href] = v8SourceMapCache[fileURL]
+      let suffix = '';
+      const match = fileURL.match(/(?<query>\?.*)$/)
+      if (match) {
+        suffix = match.groups.query;
+      }
+      cache[pathToFileURL(fileURLToPath(fileURL)).href + suffix] = v8SourceMapCache[fileURL]
     }
     return cache
   }

The approach should be fleshed out more with tests, and using a helper rather than repeated code -- also it fails if no source is found right now, since v8-to-istanbul will try to load coverage.js?=foo.bar from disk (this should probably be fixed in v8-to-istanbul, with it dropping the suffix perhaps there?).

[alecgibson]

I think we're running into this issue (or something very similar) trying to run tests with TypeScript / ESM / tsx as well: it seems to incorrectly report import statements as branches:

[iambumblehead]

It would be outstanding if c8 would differentiate moduleIds requested with query string to provide accurate coverage results.

[bcoe]

It would be outstanding if c8 would differentiate moduleIds requested with query string to provide accurate coverage results.

@iambumblehead agreed. Would happily accept a patch for this, or even a failing test.

[iambumblehead]

It would be outstanding if c8 would differentiate moduleIds requested with query string to provide accurate coverage results.

@iambumblehead agreed. Would happily accept a patch for this, or even a failing test.

@bcoe https://github.com/bcoe/c8/pull/501

[simlu]

Maintainer of several test frameworks here. Since query strings are really the only way to invalidate imports when running tests in esm, this is really not great.

Would really appreciate if someone could pick this up again! Cheers

[electrovir]

I think we're running into this issue (or something very similar) trying to run tests with TypeScript / ESM / tsx as well: it seems to incorrectly report import statements as branches:

This is happening to me as well, but only on dynamic imports.