Skip to content

Commit 716a07f

Browse files
wraithgarwraithgar
authored
fix: 100% coverage in tests (#4607)
* fix: 100% coverage in tests * 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 * fix: clean up usage output Removed usage lib, rolled logic into base-command.js Cleaned up usage output to be less redundant
1 parent 6dd1139 commit 716a07f

File tree

21 files changed

+145
-468
lines changed

21 files changed

+145
-468
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -145,22 +145,6 @@ jobs:
145145
node ./bin/npm-cli.js install --ignore-scripts --no-audit
146146
node ./bin/npm-cli.js rebuild
147147
148-
# Run the tests, but not if we're just gonna do coveralls later anyway
148+
# Run the tests
149149
- name: Run Tap tests
150-
if: matrix.platform.os != 'ubuntu-latest' || matrix.node-version != '16.x'
151-
run: node ./bin/npm-cli.js run --ignore-scripts test -- -t600 -Rbase -c
152-
env:
153-
DEPLOY_VERSION: testing
154-
155-
# Run coverage check
156-
- name: Run coverage report
157-
if: matrix.platform.os == 'ubuntu-latest' && matrix.node-version == '16.x'
158-
# turn off --check-coverage until 100%, so CI failure is relevant
159-
run: node ./bin/npm-cli.js run check-coverage -- -t600 --no-check-coverage -Rbase -c
160-
env:
161-
DEPLOY_VERSION: testing
162-
COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_OPTIONAL_TOKEN }}
163-
164-
# - name: Run sudo tests on Linux
165-
# if: matrix.os == 'ubuntu-latest'
166-
# run: sudo PATH=$PATH $(which node) . test -- --coverage --timeout 600
150+
run: node ./bin/npm-cli.js run test --ignore-scripts -- -t600 -Rbase -c

‎lib/base-command.js‎

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22

33
const { relative } = require('path')
44

5-
const usageUtil = require('./utils/usage.js')
65
const ConfigDefinitions = require('./utils/config/definitions.js')
76
const getWorkspaces = require('./workspaces/get-workspaces.js')
87

8+
const cmdAliases = require('./utils/cmd-list').aliases
9+
910
class BaseCommand {
1011
constructor (npm) {
1112
this.wrapWidth = 80
@@ -25,28 +26,43 @@ class BaseCommand {
2526
}
2627

2728
get usage () {
28-
let usage = `npm ${this.constructor.name}\n\n`
29-
if (this.constructor.description) {
30-
usage = `${usage}${this.constructor.description}\n\n`
31-
}
29+
const usage = [
30+
`${this.constructor.description}`,
31+
'',
32+
'Usage:',
33+
]
3234

33-
usage = `${usage}Usage:\n`
3435
if (!this.constructor.usage) {
35-
usage = `${usage}npm ${this.constructor.name}`
36+
usage.push(`npm ${this.constructor.name}`)
3637
} else {
37-
usage = `${usage}${this.constructor.usage
38-
.map(u => `npm ${this.constructor.name} ${u}`)
39-
.join('\n')}`
38+
usage.push(...this.constructor.usage.map(u => `npm ${this.constructor.name} ${u}`))
4039
}
4140

4241
if (this.constructor.params) {
43-
usage = `${usage}\n\nOptions:\n${this.wrappedParams}`
42+
usage.push('')
43+
usage.push('Options:')
44+
usage.push(this.wrappedParams)
45+
}
46+
47+
const aliases = Object.keys(cmdAliases).reduce((p, c) => {
48+
if (cmdAliases[c] === this.constructor.name) {
49+
p.push(c)
50+
}
51+
return p
52+
}, [])
53+
54+
if (aliases.length === 1) {
55+
usage.push('')
56+
usage.push(`alias: ${aliases.join(', ')}`)
57+
} else if (aliases.length > 1) {
58+
usage.push('')
59+
usage.push(`aliases: ${aliases.join(', ')}`)
4460
}
4561

46-
// Mostly this just appends aliases, this could be more clear
47-
usage = usageUtil(this.constructor.name, usage)
48-
usage = `${usage}\n\nRun "npm help ${this.constructor.name}" for more info`
49-
return usage
62+
usage.push('')
63+
usage.push(`Run "npm help ${this.constructor.name}" for more info`)
64+
65+
return usage.join('\n')
5066
}
5167

5268
get wrappedParams () {
@@ -69,7 +85,7 @@ class BaseCommand {
6985
if (prefix) {
7086
prefix += '\n\n'
7187
}
72-
return Object.assign(new Error(`\nUsage: ${prefix}${this.usage}`), {
88+
return Object.assign(new Error(`\n${prefix}${this.usage}`), {
7389
code: 'EUSAGE',
7490
})
7591
}

‎lib/utils/usage.js‎

Lines changed: 0 additions & 32 deletions
This file was deleted.

‎package.json‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@
216216
"licenses": "licensee --production --errors-only",
217217
"test": "tap",
218218
"test-all": "npm run test --if-present --workspaces --include-workspace-root",
219-
"check-coverage": "tap",
220219
"snap": "tap",
221220
"postsnap": "make -s mandocs",
222221
"test:nocleanup": "NO_TEST_CLEANUP=1 npm run test --",
@@ -237,7 +236,6 @@
237236
"color": 1,
238237
"files": "test/{lib,bin,index.js}",
239238
"coverage-map": "test/coverage-map.js",
240-
"check-coverage": true,
241239
"timeout": 600
242240
},
243241
"templateOSS": {

‎scripts/config-doc-command.js‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const { definitions } = require('../lib/utils/config/index.js')
2-
const usageFn = require('../lib/utils/usage.js')
2+
const cmdAliases = require('../lib/utils/cmd-list').aliases
33
const { writeFileSync, readFileSync } = require('fs')
44
const { resolve } = require('path')
55

@@ -52,9 +52,19 @@ const describeUsage = ({ usage }) => {
5252
synopsis.push(usage.map(usageInfo => `${baseCommand} ${usageInfo}`).join('\n'))
5353
}
5454

55-
const aliases = usageFn(commandName, '').trim()
56-
if (aliases) {
57-
synopsis.push(`\n${aliases}`)
55+
const aliases = Object.keys(cmdAliases).reduce((p, c) => {
56+
if (cmdAliases[c] === commandName) {
57+
p.push(c)
58+
}
59+
return p
60+
}, [])
61+
62+
if (aliases.length === 1) {
63+
synopsis.push('')
64+
synopsis.push(`alias: ${aliases[0]}`)
65+
} else if (aliases.length > 1) {
66+
synopsis.push('')
67+
synopsis.push(`aliases: ${aliases.join(', ')}`)
5868
}
5969
}
6070
} else {

0 commit comments

Comments
 (0)