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 map issued on switch statements without scoped case clauses incorrectly report an uncovered block #53652

Open
ericmorand opened this issue Jun 30, 2024 · 3 comments
Labels
coverage Issues and PRs related to native coverage support. v8 engine Issues and PRs related to the V8 dependency.

Comments

@ericmorand
Copy link

Version

v20.13.0

Platform

Linux 6.5.0-35-generic #35~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue May  7 09:00:52 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

Consider the following scripts:

  • lib.mjs
export const foo = (value) => {
    switch (value) {
        case true:
            const result = 5;
            return result;
        default:
            return 10;
    }
};
  • index.mjs
import {foo} from "./lib.mjs";

foo(false);
foo(true);

Executing the index.mjs script with node alongside the NODE_V8_COVERAGE environment variable, the output coverage map incorrectly shows the line 9 of lib.mjs (};) as uncovered.

NODE_V8_COVERAGE=./coverage node index.mjs

The coverage map file, with only the two relevant scripts for readability:

{
  "result": [
    {
      "scriptId": "100",
      "url": "file:///home/ericmorand/Projects/one-hundred-cases/src/switch-without-scoped-cases/index.mjs",
      "functions": [
        {
          "functionName": "",
          "ranges": [
            {
              "startOffset": 0,
              "endOffset": 54,
              "count": 1
            }
          ],
          "isBlockCoverage": true
        }
      ]
    },
    {
      "scriptId": "101",
      "url": "file:///home/ericmorand/Projects/one-hundred-cases/src/switch-without-scoped-cases/lib.mjs",
      "functions": [
        {
          "functionName": "",
          "ranges": [
            {
              "startOffset": 0,
              "endOffset": 178,
              "count": 1
            }
          ],
          "isBlockCoverage": true
        },
        {
          "functionName": "foo",
          "ranges": [
            {
              "startOffset": 19,
              "endOffset": 176,
              "count": 2
            },
            {
              "startOffset": 61,
              "endOffset": 128,
              "count": 1
            },
            {
              "startOffset": 137,
              "endOffset": 168,
              "count": 1
            },
            {
              "startOffset": 174,
              "endOffset": 175,
              "count": 0
            }
          ],
          "isBlockCoverage": true
        }
      ]
    }
  ]
}

How often does it reproduce? Is there a required condition?

It happens only when using either:

  • a variable assignment that is immediately returned

The following variant of lib.mjs is not impacted:

export const foo = (value) => {
    switch (value) {
        case true:
            return 5;
        default:
            return 10;
    }
};
  • unscoped case clauses

The following variant of lib.mjs is not impacted:

export const foo = (value) => {
    switch (value) {
        case true: {
            const result = 5;
            return result;
        }
        default: {
            return 10;
        }
    }
};

What is the expected behavior? Why is that the expected behavior?

The entirety of lib.mjs should be shown as covered.

What do you see instead?

The range 174:175 is considered as uncovered:

{
  "startOffset": 174,
  "endOffset": 175,
  "count": 0
}

Additional information

This issues has been impacting c8, among other code coverage tools:

bcoe/c8#497

@RedYetiDev RedYetiDev added the coverage Issues and PRs related to native coverage support. label Jun 30, 2024
@juanarbol
Copy link
Member

You should open an issue in V8. From the Node-side there’s not that much we can do.

@juanarbol juanarbol added the v8 engine Issues and PRs related to the V8 dependency. label Jun 30, 2024
@ericmorand
Copy link
Author

ericmorand commented Jun 30, 2024

@juanarbol , I agree and I would, but the only thing I know is that it fails when using Node.js: I have no idea how Node.js itself propagates this environment variable to V8. I'm afraid that if I open an issue there, explaining exactlty what I explain in the current issue, they will answer "this is a Node.js issue, open an issue to their repository".

I need to know what Node.js is doing with this environment variable so that I can open an issue at V8. And for that, I'm afraid I need your help.

@juanarbol
Copy link
Member

Node.js simply uses the V8’s inspector protocol for this. And Node.js or C8 gets all the output collected from the V8, is it not manipulated, nor the scripts are instrumented by Node.js, everything is V8, Node.js simply bypasses the output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coverage Issues and PRs related to native coverage support. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

3 participants