Help coverage instrumenters exclude compiler's helper functions

[AriPerkkio]

Describe the problem

When transpiled Svelte code is instrumented with istanbul, there are a lot of unrelated statements and branches in the coverage report. The source maps received from svelte/compiler include mappings which I think are intended for debugging breakpoints. However when we instrument the transpiled svelte code with istanbul, these unnecessary if-blocks and other statements end up getting instrumented. See https://github.com/vitest-dev/vitest/issues/1893.

For inspecting sourcemaps of transpiled and instrumented svelte file, see https://gist.github.com/AriPerkkio/8399007031cb747b2811c07358b8daa2.

Here is one example where the file is completely covered by a test. However statements and branches are not 100%. We can see uncovered code highlighted in results.

The reason why there is an if-block in middle of <li>{user}</li> is due to having an actual if-statement in transpiled code.

Those other red-highlighted blocks are marked as uncovered, as source map points to a helper function which is not executed.

Describe the proposed solution

If those source mappings are unnecessary, those could be just removed from the mappings. But as I think those are indeed used for debugging, those cannot be simply removed.

But svelte compiler could be placing hints from instrumenters. I'm 99% sure I've seen some code transformation tools placing /* istanbul ignore, c8 ignore */ comments around their internal helper functions. Could svelte do the same?

// This is the if-block from example above

p(ctx, dirty) {
  /* istanbul ignore, c8 ignore */ // <---- This would be added by svelte
  if (dirty & /*users*/ 1 && t_value !== (t_value = /*user*/ ctx[1] + "")) set_data(t, t_value);
},

Svelte is already adding some extra comments, e.g. variable names, in the transpiled code. Adding some more comments should not be an issue.

I have zero knowledge about svelte's internals so I'm not sure whether these cases can easily be identified.

Alternatives considered

I don't think any coverage tool is currently working perfectly for svelte.

Importance

nice to have

[AriPerkkio]

Here is one example in details. Complete setup for inspecting the source maps of transpiled and instrumented code can be found at https://github.com/AriPerkkio/svelte-istanbul-reproduction, especially the debug-svelte-sourcemaps.mjs part should reveal the process of code coverage.

Considering the line 13 of the following each block:

12 |  {#each users as user}
13 |    <li>{user}</li>
14 |  {/each}

When the Svelte code is transpiled to Javascript there will be following position in source map:

• #1: [24, 0, 12, 9], [27, 0, 12, 13]
• #2: [62, 0, 12, 9], [65, 0, 12, 13]

26          |  let t_value = /*user*/ ctx[1] + "";
Mapping  #1 |                         ^^^
39          |   if (dirty & /*users*/ 1 && t_value !== (t_value = /*user*/ ctx[1] + "")) set_data(t, t_value);
Mapping  #2 |                                                              ^^^

13         |    <li>{user}</li>
Mapping #1 |         ^^^^
Mapping #2 |         ^^^^

When the transpiled Javascript is instrumented, Istanbul will add the counters as below:

• #3: [51, 0, 12, 9, 0],[54, 0, 12, 13]
• #4: [122, 0, 12, 9, 0],[125, 0, 12, 13]
• #5: [8, 0, 12, 13]

1878       |  let t_value = ( /*user*/cov_208gup579f().s[3]++, ctx[1] + "");
Mapping #3 |                                                   ^^^
1899       |      if ((cov_208gup579f().b[1][0]++, dirty & /*users*/1) && (cov_208gup579f().b[1][1]++, t_value !== (t_value = /*user*/ctx[1] + ""))) {
Mapping #4 |                                                                                                                          ^^^
1900       |        cov_208gup579f().b[0][0]++;
Mapping #5 |        ^

These map to the sources as below:

13         |    <li>{user}</li>
Mapping #3 |         ^^^^
Mapping #4 |         ^^^^
Mapping #5 |            ^

And now finally in the coverage report:

13                          |    <li>{user}</li>
statementMap["0"]           |         ^^^^^^^^^^
statementMap["1"]           |    ^
statementMap["2"]           |             ^^^^^^
branchMap["0"].locations[0] |         ^^^^^^^^^^
branchMap["1"].locations[0] |         ^^^^
branchMap["1"].locations[1] |         ^^^^^^^^^^

❗ Both entries in branchMap are not present in sources but are present in transpiled code. They should be excluded from instrumentation.

So the tricky part in transpiled code is this one:

39 |   if (dirty & /*users*/ 1 && t_value !== (t_value = /*user*/ ctx[1] + "")) set_data(t, t_value);

The "Mapping pair #2" mentioned above has to be in source maps for debuggers. But that means that it will also end up in coverage reports when the preceding if (dirty & ...) gets instrumented with branch counter. Since the part cannot be removed from source maps, I think the only option is to place ignore hint before the if-block. Svelte compiler should do this.

+ 38 |   /* istanbul ignore if */
  39 |   if (dirty & /*users*/ 1 && t_value !== (t_value = /*user*/ ctx[1] + "")) set_data(t, t_value);

There are likely other similar cases that should be covered as well.

[madeleineostoja]

Is there any movement on or at least acknowledgement of this issue from anyone in the svelte team? It makes coverage reporting for tests on svelte components all but meaningless currently, I'm surprised it's not a bigger pain point for more users.

[has-macthwatch]

is there any update for this?

[jjuannn]

I'm having the same problem here guys. Is there any update for this?

[madeleineostoja]

What’s stopping those PRs being merged? If putting coverage-tool specific ignore comments in the compiled output for the two biggest tools (Istanbul and C8) is the only way to reasonably achieve this at the moment, then I think pragmatism has to win over ideals for now

[MircoSteyer]

Should we expect for Svelte5 to have the same issues?

What's the current expectation for this, given that a previous PR had been closed in favor of waiting for another major release and checking if that maybe fixes the issue. https://github.com/sveltejs/svelte/pull/8269

[Rich-Harris]

Svelte 5 has a completely different approach, which involves much less generated code with fewer if blocks and the like. I'm going to go out on a limb and mark this as completed, since it's so different, but if there's more work to be done then we'd welcome an issue with an updated repro — thanks

[AriPerkkio]

I've updated the reproduction to use Svelte 5: https://github.com/AriPerkkio/svelte-istanbul-reproduction/blob/main/debug-svelte-sourcemaps.mjs

It's definitely different. Maybe better but not perfect, e.g. line 17 is now excluded from source maps.

In the picture it says Svelte 3 but it's actually 4.

• Svelte 4: https://evanw.github.io/source-map-visualization/#MzM3MgAvKiByZXBy
• Svelte 5 https://evanw.github.io/source-map-visualization/#OTc3AC8vIHJl

On Vitest's side we'll be able to close issues like https://github.com/vitest-dev/vitest/issues/1893 now.