Skip to content

Commit

Permalink
fix: 100% coverage in tests
Browse files Browse the repository at this point in the history
 * 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
  • Loading branch information
wraithgar committed Mar 23, 2022
1 parent 362831c commit b9fc0ed
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 76 deletions.
20 changes: 2 additions & 18 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -145,22 +145,6 @@ jobs:
node ./bin/npm-cli.js install --ignore-scripts --no-audit
node ./bin/npm-cli.js rebuild
# Run the tests, but not if we're just gonna do coveralls later anyway
# Run the tests
- name: Run Tap tests
if: matrix.platform.os != 'ubuntu-latest' || matrix.node-version != '16.x'
run: node ./bin/npm-cli.js run --ignore-scripts test -- -t600 -Rbase -c
env:
DEPLOY_VERSION: testing

# Run coverage check
- name: Run coverage report
if: matrix.platform.os == 'ubuntu-latest' && matrix.node-version == '16.x'
# turn off --check-coverage until 100%, so CI failure is relevant
run: node ./bin/npm-cli.js run check-coverage -- -t600 --no-check-coverage -Rbase -c
env:
DEPLOY_VERSION: testing
COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_OPTIONAL_TOKEN }}

# - name: Run sudo tests on Linux
# if: matrix.os == 'ubuntu-latest'
# run: sudo PATH=$PATH $(which node) . test -- --coverage --timeout 600
run: node ./bin/npm-cli.js run test --ignore-scripts -- -t600 -Rbase -c
10 changes: 5 additions & 5 deletions lib/base-command.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ class BaseCommand {
}

get usage () {
let usage = `npm ${this.constructor.name}\n\n`
if (this.constructor.description) {
usage = `${usage}${this.constructor.description}\n\n`
}
let usage = [
`npm ${this.constructor.name}\n`,
`${this.constructor.description}\n`,
`Usage:\n`,
].join('\n')

usage = `${usage}Usage:\n`
if (!this.constructor.usage) {
usage = `${usage}npm ${this.constructor.name}`
} else {
Expand Down
11 changes: 2 additions & 9 deletions lib/utils/usage.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const aliases = require('./cmd-list').aliases

module.exports = function usage (cmd, txt, opt) {
module.exports = function usage (cmd, txt) {
const post = Object.keys(aliases).reduce(function (p, c) {
var val = aliases[c]
if (val !== cmd) {
Expand All @@ -9,7 +9,7 @@ module.exports = function usage (cmd, txt, opt) {
return p.concat(c)
}, [])

if (opt || post.length > 0) {
if (post.length > 0) {
txt += '\n\n'
}

Expand All @@ -21,12 +21,5 @@ module.exports = function usage (cmd, txt, opt) {
txt += post.join(', ')
}

if (opt) {
if (post.length > 0) {
txt += '\n'
}
txt += 'common options: ' + opt
}

return txt
}
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@
"licenses": "licensee --production --errors-only",
"test": "tap",
"test-all": "npm run test --if-present --workspaces --include-workspace-root",
"check-coverage": "tap",
"snap": "tap",
"postsnap": "make -s mandocs",
"test:nocleanup": "NO_TEST_CLEANUP=1 npm run test --",
Expand All @@ -235,7 +234,6 @@
"color": 1,
"files": "test/{lib,bin,index.js}",
"coverage-map": "test/coverage-map.js",
"check-coverage": true,
"timeout": 600
},
"templateOSS": {
Expand Down
29 changes: 18 additions & 11 deletions test/coverage-map.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
const full = process.env.npm_lifecycle_event === 'check-coverage'
const coverageMap = (filename) => {
if (full && /load-all.js$/.test(filename)) {
const glob = require('glob')
const { resolve, relative } = require('path')
const dir = resolve(__dirname, '../lib')
return glob.sync(`${dir}/**/*.js`)
.map(f => relative(process.cwd(), f))
const { basename } = require('path')
const testbase = basename(filename)
if (filename === 'test/index.js') {
return ['index.js']
}
if (/windows-shims\.js$/.test(filename)) {
// this one doesn't provide any coverage nyc can track
return []
if (testbase === 'load-all-commands.js') {
const { cmdList } = require('../lib/utils/cmd-list.js')
return cmdList.map(cmd => `lib/${cmd}.js`)
.concat('lib/utils/usage.js')
.concat('lib/base-command.js')
}
if (/^test\/(lib\/|bin\/|index\.js$)/.test(filename)) {
if (/^test\/lib\/commands/.test(filename) || filename === 'test/lib/npm.js') {
return [
filename.replace(/^test\//, ''),
'lib/base-command.js',
'lib/utils/usage.js',
'lib/exec/get-workspace-location-msg.js',
]
}
if (/^test\/(lib|bin)\//.test(filename)) {
return filename.replace(/^test\//, '')
}
return []
Expand Down
31 changes: 0 additions & 31 deletions test/lib/load-all.js

This file was deleted.

0 comments on commit b9fc0ed

Please sign in to comment.