From b9fc0ed4ae2ab7dd3d6e8143f58913bb47c750b8 Mon Sep 17 00:00:00 2001
From: Gar <gar+gh@danger.computer>
Date: Wed, 23 Mar 2022 07:42:27 -0700
Subject: [PATCH] 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
---
 .github/workflows/ci.yml | 20 ++------------------
 lib/base-command.js      | 10 +++++-----
 lib/utils/usage.js       | 11 ++---------
 package.json             |  2 --
 test/coverage-map.js     | 29 ++++++++++++++++++-----------
 test/lib/load-all.js     | 31 -------------------------------
 6 files changed, 27 insertions(+), 76 deletions(-)
 delete mode 100644 test/lib/load-all.js

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index b6b9125938e1e..5ebc711aefe9a 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -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
diff --git a/lib/base-command.js b/lib/base-command.js
index b6e3d6d231860..4ac370a5ebcd2 100644
--- a/lib/base-command.js
+++ b/lib/base-command.js
@@ -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 {
diff --git a/lib/utils/usage.js b/lib/utils/usage.js
index 39eaa45e4101e..8ade59140527a 100644
--- a/lib/utils/usage.js
+++ b/lib/utils/usage.js
@@ -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) {
@@ -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'
   }
 
@@ -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
 }
diff --git a/package.json b/package.json
index d51ffcd4dc4fb..b818b8c7696ed 100644
--- a/package.json
+++ b/package.json
@@ -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 --",
@@ -235,7 +234,6 @@
     "color": 1,
     "files": "test/{lib,bin,index.js}",
     "coverage-map": "test/coverage-map.js",
-    "check-coverage": true,
     "timeout": 600
   },
   "templateOSS": {
diff --git a/test/coverage-map.js b/test/coverage-map.js
index b29fcd8618557..ec7f3ae0618ee 100644
--- a/test/coverage-map.js
+++ b/test/coverage-map.js
@@ -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 []
diff --git a/test/lib/load-all.js b/test/lib/load-all.js
deleted file mode 100644
index e5d7b558c2a5b..0000000000000
--- a/test/lib/load-all.js
+++ /dev/null
@@ -1,31 +0,0 @@
-const t = require('tap')
-const glob = require('glob')
-const { resolve } = require('path')
-const { load: loadMockNpm } = require('../fixtures/mock-npm')
-
-const full = process.env.npm_lifecycle_event === 'check-coverage'
-
-if (!full) {
-  t.pass('nothing to do here, not checking for full coverage')
-} else {
-  t.test('load all', async (t) => {
-    const { npm } = await loadMockNpm(t, { })
-
-    t.teardown(() => {
-      const exitHandler = require('../../lib/utils/exit-handler.js')
-      exitHandler.setNpm(npm)
-      exitHandler()
-    })
-
-    t.test('load all the files', t => {
-      // just load all the files so we measure coverage for the missing tests
-      const dir = resolve(__dirname, '../../lib')
-      for (const f of glob.sync(`${dir}/**/*.js`)) {
-        require(f)
-        t.pass('loaded ' + f)
-      }
-      t.pass('loaded all files')
-      t.end()
-    })
-  })
-}