fix: make prefixed usage errors more consistent

[lukekarrys]

No description provided.

[wraithgar]

The reason the tests didn't fail here is because the t.rejects in the tests are only testing for the presence of certain strings in the raised exceptions. And in fact I think there's a bug in t.rejects if you pass it a string it ... just passes no matter what

~/D/n/cli (PR-3987|✔) $ tap test/lib/commands/cache.js 
test/lib/commands/cache.js .......................... 28/28
total ............................................... 28/28

  28 passing (246.797ms)

  ok
~/D/n/cli (PR-3987|✚1) $ git diff
diff --git a/test/lib/commands/cache.js b/test/lib/commands/cache.js
index c12318f4e..e73a7cd3d 100644
--- a/test/lib/commands/cache.js
+++ b/test/lib/commands/cache.js
@@ -161,7 +161,7 @@ const cache = new Cache(npm)
 t.test('cache no args', async t => {
   await t.rejects(
     cache.exec([]),
-    'usage instructions',
+    'asdf',
     'should throw usage instructions'
   )
 })

[wraithgar]

Need at least to remove this.usage from the function call. Fixing tests may be bigger than this PR since it affects more than just this one command.

[lukekarrys]

And in fact I think there's a bug in t.rejects if you pass it a string it ... just passes no matter what

If the second parameter to t.rejects is a string, then that is treated as the message:

From the docs:

t.rejects(promise | fn, [expectedError], message, extra)

I looked up the test in libtap to make sure: https://github.com/tapjs/libtap/blob/66c23ee844d28ff144c502f7be432b7141c1343c/test/test.js#L473-L475

[lukekarrys]

I opted to change this error message to be more consistent with other commands.

Here's a diff of the error messages since we aren't capturing it in a snapshot:

diff --git a/oldoutput.log b/newoutput.log
index 974aaaa34..9693e7641 100644
--- a/oldoutput.log
+++ b/newoutput.log
@@ -1,10 +1,25 @@
 npm ERR! code EUSAGE
+npm ERR! 
+npm ERR! Usage: First argument to `add` is required
+npm ERR! 
+npm ERR! npm cache
+npm ERR! 
+npm ERR! Manipulates packages cache
+npm ERR! 
 npm ERR! Usage:
-npm ERR!     npm cache add <tarball-url>...
-npm ERR!     npm cache add <pkg>@<ver>...
-npm ERR!     npm cache add <tarball>...
-npm ERR!     npm cache add <folder>...
+npm ERR! npm cache add <tarball file>
+npm ERR! npm cache add <folder>
+npm ERR! npm cache add <tarball url>
+npm ERR! npm cache add <git url>
+npm ERR! npm cache add <name>@<version>
+npm ERR! npm cache clean [<key>]
+npm ERR! npm cache ls [<name>@<version>]
+npm ERR! npm cache verify
+npm ERR! 
+npm ERR! Options:
+npm ERR! [--cache <cache>]
 npm ERR! 
+npm ERR! Run "npm help cache" for more info

[wraithgar]

I asked @lukekarrys permission to push a rather bold update to this PR, he can revert it at his whim. Usage output shouldn't look so scary. Now that we're standardizing on the error thrown we can handle it uniformly. This shows the before and after.

Before:

After:

Exit code is still 1 so this isn't a breaking change.

[wraithgar]

After discussion we'll revert my change and implement it later, it at least needs to go to stdout. At least all usage is standardized so it'll be easier to implement when we're ready