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

fix(cli/coverage): display mapped instrumentation line counts #9310

Conversation

caspervonb
Copy link
Contributor

@caspervonb caspervonb commented Jan 28, 2021

Follow up to #8995 that I didn't think about at the time.

This uses the number of original mapped source lines as the basis for the number of lines hit and found rather than just relying on the total number of lines in the instrumented script.

For example, on canary:

cover file:///home/caspervonb/deno/cli/tests/subdir/complex.ts ... 72.727% (8/11)
  25 | export function unused(
  26 |   foo: string,
  27 |   bar: string,
  28 |   baz: string,
-----|-----
  30 |   return complex(
  31 |     foo,
  32 |     bar,
  33 |     baz,
  34 |   );
  35 | }

This patch (the number of missing lines are matching with the number of lines rendered as missing):

cover file:///home/caspervonb/deno/cli/tests/subdir/complex.ts ... 50.000% (10/20)
  25 | export function unused(
  26 |   foo: string,
  27 |   bar: string,
  28 |   baz: string,
-----|-----
  30 |   return complex(
  31 |     foo,
  32 |     bar,
  33 |     baz,
  34 |   );
  35 | }

@caspervonb caspervonb force-pushed the fix-cli-coverage-display-instrumented-line-counts branch from b28e330 to 7cabe77 Compare January 28, 2021 19:03
@caspervonb caspervonb force-pushed the fix-cli-coverage-display-instrumented-line-counts branch from 7cabe77 to d2e4af6 Compare January 28, 2021 19:48
@caspervonb caspervonb marked this pull request as ready for review January 28, 2021 20:18
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks @caspervonb - coverage is getting quite mature now.

@ry ry merged commit 9965fc8 into denoland:master Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants