Skip to content

fix: 100% coverage in tests - #4607

Merged
fritzy merged 2 commits into
latestfrom
gar/coverage-map-update
Mar 24, 2022
Merged

fritzy merged 2 commits into
latestfrom
gar/coverage-map-update

Conversation

@wraithgar

@wraithgar wraithgar commented Mar 23, 2022 •

Copy link
Copy Markdown
Contributor
  • Removed dead code in lib/utils/usage.js.
  • Removed dead code in lib/base-command.js.
  • Removed "load-all" test, we currently have 100% coverage and new PRs
    without tests will be rejected if they don't add coverage for new
    files.
  • Removed check-coverage script as a separate command.
  • Removed separate coverage test in ci.yml.
  • Removed coverage flag from tap config, the default is already to
    enforce 100% coverage.

Added another commit that cleans up usage, which removing the dead code from base-command revealed as being needed.

  • Removed usage lib, rolled logic into base-command.js
  • Cleaned up usage output to be less redundant

@wraithgar
wraithgar requested a review from a team as a code owner March 23, 2022 18:22
@npm-robot

npm-robot commented Mar 23, 2022 •

Copy link
Copy Markdown
Contributor

found 1 benchmarks with statistically significant performance regressions

  • app-large: no-clean:audit
timing results
app-large clean lock-only cache-only cache-only
peer-deps
modules-only no-lock no-cache no-modules no-clean no-clean
audit
npm@8 54.614 ±2.72 36.328 ±0.73 22.541 ±0.57 25.401 ±1.45 3.528 ±0.05 3.611 ±0.00 2.925 ±0.09 14.539 ±0.01 2.855 ±0.03 3.980 ±0.02
#4607 56.995 ±0.58 36.619 ±0.05 22.444 ±0.53 26.387 ±0.83 3.681 ±0.02 3.749 ±0.11 2.924 ±0.02 15.505 ±0.31 2.969 ±0.06 4.521 ±0.31
app-medium clean lock-only cache-only cache-only
peer-deps
modules-only no-lock no-cache no-modules no-clean no-clean
audit
npm@8 39.577 ±0.37 27.423 ±0.52 15.887 ±0.58 17.303 ±0.39 3.297 ±0.05 3.353 ±0.05 2.993 ±0.07 11.237 ±0.27 2.744 ±0.02 3.776 ±0.02
#4607 41.724 ±0.71 28.645 ±0.07 16.589 ±0.01 18.132 ±0.15 3.346 ±0.04 3.403 ±0.07 2.987 ±0.04 11.062 ±0.17 2.743 ±0.01 3.821 ±0.03

@wraithgar
wraithgar force-pushed the gar/coverage-map-update branch from 67b44a7 to 655aec8 Compare March 23, 2022 19:58
@wraithgar wraithgar mentioned this pull request Mar 23, 2022
@ruyadorno ruyadorno added semver:patch semver patch level for changes Release 8.x work is associated with a specific npm 8 release Release-Next labels Mar 24, 2022
 * Removed dead code in `lib/utils/usage.js`.
 * Removed dead code in `lib/base-command.js`.
 * Removed "load-all" test, we currently have 100% coverage and new PRs
   without tests will be rejected if they don't add coverage for new
   files.
 * Removed `check-coverage` script as a separate command.
 * Removed separate coverage test in ci.yml.
 * Removed `coverage` flag from tap config, the default is already to
   enforce 100% coverage.

Removed a tiny bit of dead code resulting from this
@wraithgar
wraithgar force-pushed the gar/coverage-map-update branch from 655aec8 to 5649c30 Compare March 24, 2022 16:11
Removed usage lib, rolled logic into base-command.js
Cleaned up usage output to be less redundant
@wraithgar
wraithgar force-pushed the gar/coverage-map-update branch from 5649c30 to 38a97e6 Compare March 24, 2022 16:15
@fritzy
fritzy merged commit 716a07f into latest Mar 24, 2022
@fritzy
fritzy deleted the gar/coverage-map-update branch March 24, 2022 20:23
@lukekarrys lukekarrys mentioned this pull request Mar 31, 2022
@lukekarrys lukekarrys mentioned this pull request Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Release 8.x work is associated with a specific npm 8 release semver:patch semver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants