fix(usage): clean up usage declarations

Small refactor of commands to allow usage to be more programmatically
generated, leading us in the direction of more tighly coupling each
command to the params it accepts.

PR-URL: https://github.com/npm/cli/pull/2821
Credit: @wraithgar
Close: #2821
Reviewed-by: @isaacs

15 files changed, 33 insertions, 54 deletions

diff --git a/test/lib/access.js b/test/lib/access.js
index 8134c1d91..a8e28c8eb 100644
--- a/test/lib/access.js
+++ b/test/lib/access.js
@@ -41,7 +41,7 @@ test('completion', t => {
 test('subcommand required', t => {
   const access = new Access({ flatOptions: {} })
   access.exec([], (err) => {
-    t.equal(err, '\nUsage: Subcommand is required.\n\n' + access.usage)
+    t.match(err, access.usageError('Subcommand is required.'))
     t.end()
   })
 })
diff --git a/test/lib/adduser.js b/test/lib/adduser.js
index 106cd429e..d9106e1bd 100644
--- a/test/lib/adduser.js
+++ b/test/lib/adduser.js
@@ -78,6 +78,10 @@ const AddUser = requireInject('../../lib/adduser.js', {
 
 const adduser = new AddUser(npm)
 
+test('usage', (t) => {
+  t.match(adduser.usage, 'adduser', 'usage has command name in it')
+  t.end()
+})
 test('simple login', (t) => {
   adduser.exec([], (err) => {
     t.ifError(err, 'npm adduser')
diff --git a/test/lib/bin.js b/test/lib/bin.js
index 512fa8d02..428b2e3ba 100644
--- a/test/lib/bin.js
+++ b/test/lib/bin.js
@@ -2,7 +2,7 @@ const { test } = require('tap')
 const requireInject = require('require-inject')
 
 test('bin', (t) => {
-  t.plan(3)
+  t.plan(4)
   const dir = '/bin/dir'
 
   const Bin = require('../../lib/bin.js')
@@ -15,6 +15,7 @@ test('bin', (t) => {
     },
   }
   const bin = new Bin(npm)
+  t.match(bin.usage, 'bin', 'usage has command name in it')
 
   bin.exec([], (err) => {
     t.ifError(err, 'npm bin')
diff --git a/test/lib/bugs.js b/test/lib/bugs.js
index e98131f11..21b1a9841 100644
--- a/test/lib/bugs.js
+++ b/test/lib/bugs.js
@@ -55,6 +55,11 @@ const Bugs = requireInject('../../lib/bugs.js', {
 
 const bugs = new Bugs({ flatOptions: {} })
 
+t.test('usage', (t) => {
+  t.match(bugs.usage, 'bugs', 'usage has command name in it')
+  t.end()
+})
+
 t.test('open bugs urls', t => {
   const expect = {
     nobugs: 'https://www.npmjs.com/package/nobugs',
diff --git a/test/lib/cache.js b/test/lib/cache.js
index 5c2588f34..773adc6a8 100644
--- a/test/lib/cache.js
+++ b/test/lib/cache.js
@@ -70,7 +70,7 @@ const cache = new Cache(npm)
 
 t.test('cache no args', t => {
   cache.exec([], err => {
-    t.equal(err.message, 'usage instructions', 'should throw usage instructions')
+    t.match(err.message, 'usage instructions', 'should throw usage instructions')
     t.end()
   })
 })
diff --git a/test/lib/config.js b/test/lib/config.js
index 48934ba4e..3aeb29f8d 100644
--- a/test/lib/config.js
+++ b/test/lib/config.js
@@ -180,12 +180,7 @@ t.test('config list --json', t => {
 
 t.test('config delete no args', t => {
   config.exec(['delete'], (err) => {
-    t.equal(
-      err.message,
-      'usage instructions',
-      'should throw usage error'
-    )
-    t.equal(err.code, 'EUSAGE', 'should throw expected error code')
+    t.match(err, { message: '\nUsage: usage instructions' })
     t.end()
   })
 })
@@ -265,11 +260,7 @@ t.test('config delete key --global', t => {
 
 t.test('config set no args', t => {
   config.exec(['set'], (err) => {
-    t.equal(
-      err.message,
-      'usage instructions',
-      'should throw usage error'
-    )
+    t.match(err, { message: '\nUsage: usage instructions' })
     t.end()
   })
 })
diff --git a/test/lib/deprecate.js b/test/lib/deprecate.js
index 03100166a..e278a3e3c 100644
--- a/test/lib/deprecate.js
+++ b/test/lib/deprecate.js
@@ -60,14 +60,14 @@ test('completion', async t => {
 
 test('no args', t => {
   deprecate.exec([], (err) => {
-    t.match(err, /Usage: npm deprecate/, 'logs usage')
+    t.match(err, 'Usage:', 'logs usage')
     t.end()
   })
 })
 
 test('only one arg', t => {
   deprecate.exec(['foo'], (err) => {
-    t.match(err, /Usage: npm deprecate/, 'logs usage')
+    t.match(err, 'Usage:', 'logs usage')
     t.end()
   })
 })
diff --git a/test/lib/explore.js b/test/lib/explore.js
index af16444ca..a0655380e 100644
--- a/test/lib/explore.js
+++ b/test/lib/explore.js
@@ -338,7 +338,7 @@ t.test('usage if no pkg provided', t => {
   for (const args of noPkg) {
     t.test(JSON.stringify(args), t => {
       posixExplore.exec(args, er => {
-        t.equal(er, 'npm explore <pkg> [ -- <command>]')
+        t.match(er, 'Usage:')
         t.strictSame({
           ERROR_HANDLER_CALLED: null,
           RPJ_CALLED,
diff --git a/test/lib/load-all-commands.js b/test/lib/load-all-commands.js
index f6d1ae9e1..e31a2b993 100644
--- a/test/lib/load-all-commands.js
+++ b/test/lib/load-all-commands.js
@@ -1,6 +1,6 @@
 // Thanks to nyc not working properly with proxies this
-// doesn't affect coverage. but it does ensure that every command
-// has a usage, and if it has completion it is a function
+// doesn't affect coverage. but it does ensure that every command has a usage
+// that contains its name, and if it has completion it is a function
 const npm = require('../../lib/npm.js')
 const t = require('tap')
 const { cmdList } = require('../../lib/utils/cmd-list.js')
diff --git a/test/lib/npm.js b/test/lib/npm.js
index 1f7a54e22..87cbea8f2 100644
--- a/test/lib/npm.js
+++ b/test/lib/npm.js
@@ -357,7 +357,7 @@ t.test('loading as main will load the cli', t => {
   p.on('close', (code, signal) => {
     t.equal(code, 0)
     t.equal(signal, null)
-    t.equal(Buffer.concat(out).toString().trim(), ls.usage)
+    t.match(Buffer.concat(out).toString(), ls.usage)
     t.end()
   })
 })
diff --git a/test/lib/owner.js b/test/lib/owner.js
index 14753689f..11aeacb7b 100644
--- a/test/lib/owner.js
+++ b/test/lib/owner.js
@@ -42,11 +42,7 @@ t.test('owner no args', t => {
   })
 
   owner.exec([], err => {
-    t.equal(
-      err.message,
-      'usage instructions',
-      'should throw usage instructions'
-    )
+    t.match(err, /usage instructions/, 'should not error out on empty locations')
     t.end()
   })
 })
@@ -89,11 +85,7 @@ t.test('owner ls no args no cwd package', t => {
   })
 
   owner.exec(['ls'], err => {
-    t.equal(
-      err.message,
-      'usage instructions',
-      'should throw usage instructions if no cwd package available'
-    )
+    t.match(err, /usage instructions/, 'should throw usage instructions if no cwd package available')
     t.end()
   })
 })
@@ -469,11 +461,7 @@ t.test('owner add no user', t => {
   })
 
   owner.exec(['add'], err => {
-    t.equal(
-      err.message,
-      'usage instructions',
-      'should throw usage instructions if no user provided'
-    )
+    t.match(err, /usage instructions/, 'should throw usage instructions if user provided')
     t.end()
   })
 })
@@ -485,11 +473,7 @@ t.test('owner add <user> no cwd package', t => {
   })
 
   owner.exec(['add', 'foo'], err => {
-    t.equal(
-      err.message,
-      'usage instructions',
-      'should throw usage instructions if no user provided'
-    )
+    t.match(err, /usage instructions/, 'should throw usage instructions if no user provided')
     t.end()
   })
 })
@@ -676,11 +660,7 @@ t.test('owner rm no user', t => {
   })
 
   owner.exec(['rm'], err => {
-    t.equal(
-      err.message,
-      'usage instructions',
-      'should throw usage instructions if no user provided to rm'
-    )
+    t.match(err, /usage instructions/, 'should throw usage instructions if no user provided to rm')
     t.end()
   })
 })
@@ -692,11 +672,7 @@ t.test('owner rm <user> no cwd package', t => {
   })
 
   owner.exec(['rm', 'foo'], err => {
-    t.equal(
-      err.message,
-      'usage instructions',
-      'should throw usage instructions if no user provided to rm'
-    )
+    t.match(err, /usage instructions/, 'should throw usage instructions if no user provided to rm')
     t.end()
   })
 })
diff --git a/test/lib/restart.js b/test/lib/restart.js
index f29592d9b..9719476c4 100644
--- a/test/lib/restart.js
+++ b/test/lib/restart.js
@@ -10,7 +10,6 @@ const npm = {
 }
 const Restart = require('../../lib/restart.js')
 const restart = new Restart(npm)
-t.equal(restart.usage, 'npm restart [-- <args>]')
 restart.exec(['foo'], () => {
   t.match(runArgs, ['restart', 'foo'])
   t.end()
diff --git a/test/lib/start.js b/test/lib/start.js
index 9a3328309..4e77f9691 100644
--- a/test/lib/start.js
+++ b/test/lib/start.js
@@ -10,7 +10,6 @@ const npm = {
 }
 const Start = require('../../lib/start.js')
 const start = new Start(npm)
-t.equal(start.usage, 'npm start [-- <args>]')
 start.exec(['foo'], () => {
   t.match(runArgs, ['start', 'foo'])
   t.end()
diff --git a/test/lib/stop.js b/test/lib/stop.js
index e6cb193b6..92ca84bd8 100644
--- a/test/lib/stop.js
+++ b/test/lib/stop.js
@@ -10,7 +10,6 @@ const npm = {
 }
 const Stop = require('../../lib/stop.js')
 const stop = new Stop(npm)
-t.equal(stop.usage, 'npm stop [-- <args>]')
 stop.exec(['foo'], () => {
   t.match(runArgs, ['stop', 'foo'])
   t.end()
diff --git a/test/lib/utils/lifecycle-cmd.js b/test/lib/utils/lifecycle-cmd.js
index 2f1f693f2..3e3a7da43 100644
--- a/test/lib/utils/lifecycle-cmd.js
+++ b/test/lib/utils/lifecycle-cmd.js
@@ -10,7 +10,12 @@ const npm = {
   },
 }
 t.test('create a lifecycle command', t => {
-  const cmd = new LifecycleCmd(npm, 'test-stage')
+  class TestStage extends LifecycleCmd {
+    static get name () {
+      return 'test-stage'
+    }
+  }
+  const cmd = new TestStage(npm)
   t.match(cmd.usage, /test-stage/)
   cmd.exec(['some', 'args'], (er, result) => {
     t.same(runArgs, ['test-stage', 'some', 'args'])