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

missing coverage of closing brace when return ends a switch/case #81

Closed
Trott opened this issue Apr 24, 2019 · 10 comments
Closed

missing coverage of closing brace when return ends a switch/case #81

Trott opened this issue Apr 24, 2019 · 10 comments
Labels
help wanted Issue is well defined but needs assignment

Comments

@Trott
Copy link
Contributor

Trott commented Apr 24, 2019

  • Version: tested on 8.15.1, 10.15.3, 11.14.0, 12.0.0, 13.0.0-pre
  • Platform: Darwin Fhqwhgads.local 17.7.0 Darwin Kernel Version 17.7.0: Wed Feb 27 00:43:23 PST 2019; root:xnu-4570.71.35~1/RELEASE_X86_64 x86_64

Code in foo.js:

function checkIt(arg) {
  switch (arg) {
    case true:
      return 'It was true!';
    case false:
      return 'It as not true';
  }
  return 'It was neither true nor false';
}

const val1 = checkIt(true);
const val2 = checkIt(false);
const val3 = checkIt('string');

console.log(val1);
console.log(val2);
console.log(val3);

Command:

c8 node foo.js

Output:

It was true!
It as not true
It was neither true nor false
----------|----------|----------|----------|----------|-------------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------|----------|----------|----------|----------|-------------------|
All files |    94.12 |    71.43 |      100 |    94.12 |                   |
 foo.js   |    94.12 |    71.43 |      100 |    94.12 |                 7 |

Expected output:

I expected to see 100% coverage...
@bcoe
Copy link
Owner

bcoe commented Apr 29, 2019

It was true!
It as not true
It was neither true nor false
----------|----------|----------|----------|----------|-------------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------|----------|----------|----------|----------|-------------------|
All files |    94.12 |    71.43 |      100 |    94.12 |                   |
 foo.js   |    94.12 |    71.43 |      100 |    94.12 |                 7 |
----------|----------|----------|----------|----------|-------------------|

I'm seeing line 7 missing coverage, which seems potentially like a bug in V8 -- I think that there's a chance your 0% coverage is a configuration issue; can you share a repo with this bug?

@bcoe bcoe added the help wanted Issue is well defined but needs assignment label Apr 29, 2019
@Trott
Copy link
Contributor Author

Trott commented Apr 29, 2019

@bcoe If I rename the file to foo.js, I get the results you show. I get 0% coverage if the file is named test.js which I guess is by design? Happy to provide a repo if it helps, but honestly, it's just the file contents shown above and nothing else.

@shinnn
Copy link
Contributor

shinnn commented Apr 29, 2019

which I guess is by design?

Yes.

$ c8 --help
[...]
--exclude, -x      a list of specific files and directories that should be
                     excluded from coverage (glob patterns are supported)
                                                                    [Default:
  ["coverage/**","packages/*/test/**","test/**","test{,-*}.js","**/*{.,-}test.js
                 ","**/__tests__/**","**/node_modules/**","**/babel.config.js"]]

One of the default glob patterns test{,-*}.js matched test.js and it was excluded by chance.

@bcoe
Copy link
Owner

bcoe commented Apr 29, 2019

@Trott do we want to keep this open still? but perhaps be more specific, like closing } of switch statement is not covered?

@Trott
Copy link
Contributor Author

Trott commented Apr 30, 2019

@Trott do we want to keep this open still? but perhaps be more specific, like closing } of switch statement is not covered?

¯\(ツ)/¯ Seems like it might have the same root cause as #61 and/or #62? So if you wanted to close it as a probable-duplicate, I would be OK with that. But in the meantime, I'll update it....

@Trott Trott changed the title missing coverage with return inside switch/case missing coverage of closing brace when return ends a switch/case Apr 30, 2019
@Trott
Copy link
Contributor Author

Trott commented Apr 30, 2019

(Updated. Might not be a bad idea to hide most of the subsequent comments as outdated or whatever.)

@bcoe
Copy link
Owner

bcoe commented Apr 30, 2019

@Trott thanks 👍 ... these bugs will probably ultimately be addressed in V8, but I'm happy to track here for the time being.

@Oloompa
Copy link

Oloompa commented Sep 6, 2019

i meet similar issue

foobar.ts

export function test_1(param: boolean) {
    switch (param) {
        case true:
            console.log("true");
            break;
        case false:
            console.log("false");
    }
}

export function test_2(param: boolean) {
    switch (param) {
        case true:
            console.log("true");
            break;
        default:
            console.log("false");
            break;
    }
}

export function test_3(param: boolean) {
    switch (param) {
        case true:
            console.log("true");
            break;
        default:
            throw new Error("error");
    }
}

test.ts

import { test_1, test_2, test_3 } from "./foobar";

test_1(true);
test_1(false);
test_2(true);
test_2(false);
test_3(true);
try {
    test_3(false);
} catch (err) {
    console.log("error caught");
}

output

true
false
true
false
true
error caught

----------|----------|----------|----------|----------|-------------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------|----------|----------|----------|----------|-------------------|
All files |       80 |    66.67 |      100 |       80 |                   |
 plop.ts  |       80 |    66.67 |      100 |       80 | 16,19,20,27,29,30 |
----------|----------|----------|----------|----------|-------------------|

In VS Code
image

It not only happens with return in switch statement.
it happens also if last item before braces is a break;
it happens also if last item before braces is a throw

It happens on line with default too

This make border effects too for line 20 and 30 on export close bracket - due to previous issues my opinion

@Trott
Copy link
Contributor Author

Trott commented Sep 19, 2019

I think this has been fixed in V8 although awaiting a release?

@bcoe
Copy link
Owner

bcoe commented Sep 19, 2019

@Trott hey, I remember this bit of the codebase,

v8/v8@deac757

Let's cherry-pick that commit 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issue is well defined but needs assignment
Projects
None yet
Development

No branches or pull requests

4 participants