Skip to content

Commit

Permalink
fix: 100% coverage in tests (#4607)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
wraithgar authored Mar 24, 2022
1 parent 6dd1139 commit 716a07f
Show file tree
Hide file tree
Showing 21 changed files with 145 additions and 468 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
48 changes: 32 additions & 16 deletions lib/base-command.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

const { relative } = require('path')

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

const cmdAliases = require('./utils/cmd-list').aliases

class BaseCommand {
constructor (npm) {
this.wrapWidth = 80
Expand All @@ -25,28 +26,43 @@ class BaseCommand {
}

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

usage = `${usage}Usage:\n`
if (!this.constructor.usage) {
usage = `${usage}npm ${this.constructor.name}`
usage.push(`npm ${this.constructor.name}`)
} else {
usage = `${usage}${this.constructor.usage
.map(u => `npm ${this.constructor.name} ${u}`)
.join('\n')}`
usage.push(...this.constructor.usage.map(u => `npm ${this.constructor.name} ${u}`))
}

if (this.constructor.params) {
usage = `${usage}\n\nOptions:\n${this.wrappedParams}`
usage.push('')
usage.push('Options:')
usage.push(this.wrappedParams)
}

const aliases = Object.keys(cmdAliases).reduce((p, c) => {
if (cmdAliases[c] === this.constructor.name) {
p.push(c)
}
return p
}, [])

if (aliases.length === 1) {
usage.push('')
usage.push(`alias: ${aliases.join(', ')}`)
} else if (aliases.length > 1) {
usage.push('')
usage.push(`aliases: ${aliases.join(', ')}`)
}

// Mostly this just appends aliases, this could be more clear
usage = usageUtil(this.constructor.name, usage)
usage = `${usage}\n\nRun "npm help ${this.constructor.name}" for more info`
return usage
usage.push('')
usage.push(`Run "npm help ${this.constructor.name}" for more info`)

return usage.join('\n')
}

get wrappedParams () {
Expand All @@ -69,7 +85,7 @@ class BaseCommand {
if (prefix) {
prefix += '\n\n'
}
return Object.assign(new Error(`\nUsage: ${prefix}${this.usage}`), {
return Object.assign(new Error(`\n${prefix}${this.usage}`), {
code: 'EUSAGE',
})
}
Expand Down
32 changes: 0 additions & 32 deletions lib/utils/usage.js

This file was deleted.

2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,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 @@ -237,7 +236,6 @@
"color": 1,
"files": "test/{lib,bin,index.js}",
"coverage-map": "test/coverage-map.js",
"check-coverage": true,
"timeout": 600
},
"templateOSS": {
Expand Down
18 changes: 14 additions & 4 deletions scripts/config-doc-command.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const { definitions } = require('../lib/utils/config/index.js')
const usageFn = require('../lib/utils/usage.js')
const cmdAliases = require('../lib/utils/cmd-list').aliases
const { writeFileSync, readFileSync } = require('fs')
const { resolve } = require('path')

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

const aliases = usageFn(commandName, '').trim()
if (aliases) {
synopsis.push(`\n${aliases}`)
const aliases = Object.keys(cmdAliases).reduce((p, c) => {
if (cmdAliases[c] === commandName) {
p.push(c)
}
return p
}, [])

if (aliases.length === 1) {
synopsis.push('')
synopsis.push(`alias: ${aliases[0]}`)
} else if (aliases.length > 1) {
synopsis.push('')
synopsis.push(`aliases: ${aliases.join(', ')}`)
}
}
} else {
Expand Down
Loading

0 comments on commit 716a07f

Please sign in to comment.