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.

Sign up for GitHub

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

Jump to bottom

chore: add script to check unit test coverage #4076

Closed
wants to merge 2 commits into from
Closed

chore: add script to check unit test coverage #4076

wants to merge 2 commits into from

Conversation

HyunSangHan
Copy link
Contributor

@HyunSangHan HyunSangHan commented Oct 19, 2019 •
edited
Loading

Description of the Change

Add script npm start 'unit-coverage-report' to check unit test coverage of Mocha.
By using the script, unit test coverage can be checked in terminal like below:

=============================== Coverage summary ===============================
Statements   : 63.52% ( 1858/2925 )
Branches     : 57.73% ( 672/1164 )
Functions    : 50.86% ( 266/523 )
Lines        : 63.67% ( 1851/2907 )
================================================================================
-------------------------|----------|----------|----------|----------|-------------------|
File                     |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-------------------------|----------|----------|----------|----------|-------------------|
All files                |    63.52 |    57.73 |    50.86 |    63.67 |                   |
 mocha                   |     87.5 |       50 |      100 |     87.5 |                   |
  index.js               |      100 |      100 |      100 |      100 |                   |
  package-scripts.js     |    85.71 |       50 |      100 |    85.71 |                16 |
 mocha/lib               |    86.77 |    79.03 |    85.53 |    86.86 |                   |
  context.js             |    92.86 |       90 |      100 |    92.86 |            99,100 |
  errors.js              |    58.82 |        0 |       50 |    58.82 |... 11,112,113,114 |
  growl.js               |    42.42 |    33.33 |     37.5 |    42.42 |... 99,102,103,114 |
  hook.js                |    91.67 |       50 |      100 |    91.67 |                45 |
  mocha.js               |    92.02 |    78.22 |    88.57 |    92.02 |... 43,799,805,808 |
  pending.js             |      100 |      100 |      100 |      100 |                   |
  runnable.js            |    93.89 |     90.2 |     93.1 |    93.89 |... 44,333,402,427 |
  runner.js              |    81.55 |    69.84 |    85.19 |     81.5 |... 34,968,975,981 |
  stats-collector.js     |     93.1 |       75 |     87.5 |     93.1 |             54,69 |
  suite.js               |    97.79 |    85.14 |     97.3 |    97.77 |... 66,314,434,435 |
  test.js                |      100 |      100 |      100 |      100 |                   |
  utils.js               |    81.48 |    84.38 |    79.55 |    82.24 |... 78,590,591,830 |
 mocha/lib/cli           |     65.2 |    61.99 |    46.34 |     65.5 |                   |
  cli.js                 |       75 |       40 |       50 |       75 |    49,50,51,52,73 |
  collect-files.js       |    67.57 |    30.77 |    57.14 |    70.59 |... 62,72,75,76,80 |
  commands.js            |      100 |      100 |      100 |      100 |                   |
  config.js              |    71.43 |    61.54 |       50 |    71.43 |... 48,49,50,52,57 |
  init.js                |    38.89 |      100 |        0 |    38.89 |... 30,33,34,35,36 |
  node-flags.js          |      100 |    93.33 |      100 |      100 |                78 |
  one-and-dones.js       |    31.25 |        0 |        0 |    33.33 |... 31,32,38,52,59 |
  options.js             |    94.29 |    86.36 |     87.5 |    94.17 |... 31,134,135,140 |
  run-helpers.js         |    64.29 |    60.71 |    66.67 |    65.45 |... 70,171,173,176 |
  run-option-metadata.js |      100 |      100 |      100 |      100 |                   |
  run.js                 |    68.57 |    22.22 |    57.14 |    68.57 |... 68,269,275,283 |
  watch-run.js           |    16.42 |        0 |        0 |    16.42 |... 58,167,176,184 |
 mocha/lib/interfaces    |    38.36 |    10.64 |    26.32 |    38.36 |                   |
  bdd.js                 |    81.25 |       50 |    44.44 |    81.25 |... ,85,98,106,113 |
  common.js              |    48.72 |    11.54 |    42.86 |    48.72 |... 68,169,178,187 |
  exports.js             |       16 |        0 |        0 |       16 |... 48,51,52,53,54 |
  index.js               |      100 |      100 |      100 |      100 |                   |
  qunit.js               |    13.79 |        0 |        0 |    13.79 |... 83,90,91,94,95 |
  tdd.js                 |    13.33 |        0 |        0 |    13.33 |... ,97,98,101,102 |
 mocha/lib/reporters     |    32.67 |    13.48 |     9.58 |    32.84 |                   |
  base.js                |    34.72 |    20.22 |    14.81 |    34.97 |... 57,471,474,493 |
  doc.js                 |       25 |        0 |        0 |       25 |... 69,70,75,76,81 |
  dot.js                 |    32.35 |        0 |        0 |    32.35 |... 65,67,70,71,72 |
  html.js                |     12.5 |        0 |        0 |    12.58 |... 75,383,384,386 |
  index.js               |      100 |      100 |      100 |      100 |                   |
  json-stream.js         |    33.33 |        0 |        0 |    33.33 |... 50,53,54,70,82 |
  json.js                |    21.43 |        0 |        0 |    21.43 |... 28,129,130,132 |
  landing.js             |    33.33 |        0 |        0 |    33.33 |... 93,96,97,98,99 |
  list.js                |    43.75 |      100 |        0 |    43.75 |... 62,65,66,67,70 |
  markdown.js            |    19.61 |        0 |        0 |    19.61 |... 05,106,107,108 |
  min.js                 |    61.54 |      100 |        0 |    61.54 |    35,37,39,41,44 |
  nyan.js                |    17.21 |        0 |        0 |    17.65 |... 62,263,264,273 |
  progress.js            |       25 |        0 |        0 |       25 |... 86,92,93,94,95 |
  spec.js                |     97.5 |      100 |     87.5 |     97.5 |                88 |
  tap.js                 |    27.16 |        0 |        0 |    27.16 |... 77,278,280,285 |
  xunit.js               |    67.12 |    41.03 |    45.45 |    67.12 |... 82,183,185,211 |
-------------------------|----------|----------|----------|----------|-------------------|

Alternate Designs

The way to visualize seems ultimately better to be Coveralls.

Benefits

Can easily check unit test coverage after writing moar tests to increase it.

Possible Drawbacks

None to knowledge

Applicable issues

Partial #3601

boneskull
boneskull suggested changes Nov 22, 2019
View reviewed changes
Copy link
Contributor

@boneskull boneskull left a comment

Choose a reason for hiding this comment

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

Thanks. You can see in package-scripts.js that we use a commands created by the presence of the COVERAGE env var.

This is actually a set of many test runs, all of which generate coverage. The coverage data is then merged together to create the report.

  1. Please rename the script coverage
  2. Change the script to be cross-env COVERAGE=1 nps test.node && nyc report --reporter=text. This will set the COVERAGE env var, run all the Node.js tests, then output a text report.

@boneskull boneskull added area: async related to asynchronous use of Mocha semver-patch implementation requires increase of "patch" version number; "bug fixes" labels Nov 22, 2019
@HyunSangHan
Reflect a feedback
faaf42d 
Rename the script to coverage and change the script
@HyunSangHan
Copy link
Contributor Author

@boneskull
Thanks for the detailed advice! Reflected feedback.

@outsideris outsideris added qa and removed area: async related to asynchronous use of Mocha labels Dec 14, 2019
@HyunSangHan
Copy link
Contributor Author

HyunSangHan commented Jun 8, 2021 •
edited
Loading

@boneskull Could you please take a look at this PR? It has been approved but not yet merged. :)

cc / @craigtaub @outsideris @juergba

@JoshuaKGoldberg
Copy link
Member

👋 coming back to this @HyunSangHan, are you still interested in working on this PR? As of #5027 there are again maintainers who can review it now. No worries if you no longer have time - but if you do that'd be great!

(I do see that your last message was asking for review - unless directed otherwise we'll try to review it once we're ramped up enough!)

@HyunSangHan
Copy link
Contributor Author

@JoshuaKGoldberg
Hi! I am glad to see your comment! I am still interested in this PR.
I would appreciate it if you could review this PR.
Thanks.

@JoshuaKGoldberg JoshuaKGoldberg added area: repository tooling concerning ease of contribution and removed area: qa labels Jan 2, 2024
JoshuaKGoldberg
JoshuaKGoldberg approved these changes Mar 1, 2024
View reviewed changes
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

LGTM! I think we can figure out as a followup what, if anything, to do with this nice new report. Thanks! 🚀

Leaving open for @mochajs/maintenance-crew to take a look too.

@JoshuaKGoldberg JoshuaKGoldberg changed the title Add script to check unit test coverage chore: add script to check unit test coverage Mar 4, 2024
@JoshuaKGoldberg JoshuaKGoldberg added the status: in triage a maintainer should (re-)triage (review) this issue label Mar 4, 2024
voxpelli
voxpelli reviewed Jun 4, 2024
View reviewed changes
Copy link
Member

@voxpelli voxpelli left a comment

Choose a reason for hiding this comment

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

I personally would rather always generate both text and html coverage (nyc --reporter=lcov --reporter=text) and replace nyc with c8 and replace nps with npm scripts (#5126)

Merging this now and then removing it later as part of eg. #5126 revamp feels a bit redundant, but I will not block it.

@voxpelli
Copy link
Member

voxpelli commented Aug 6, 2024

This is no longer applicable after merging #5126, and is now kind of the default mode:

mocha/package.json

Line 87 in a777fd1

"test-node": "run-s test-coverage-clean test-node:* test-coverage-generate",

mocha/package.json

Line 69 in a777fd1

"test-coverage-generate": "nyc report --reporter=lcov --reporter=text",

@voxpelli voxpelli closed this Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@voxpelli voxpelli voxpelli left review comments

@outsideris outsideris outsideris approved these changes

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg approved these changes

@craigtaub craigtaub craigtaub approved these changes

@boneskull boneskull Awaiting requested review from boneskull

Assignees
No one assigned
Labels
area: repository tooling concerning ease of contribution semver-patch implementation requires increase of "patch" version number; "bug fixes" status: in triage a maintainer should (re-)triage (review) this issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@HyunSangHan @JoshuaKGoldberg @voxpelli @outsideris @boneskull @craigtaub