feat(workspaces): add repo and docs

This adds workspaces support to `npm repo` and `npm docs`.

It also updates the usage output to support the -w and -ws parameters
output, and cleans up some unneccessary functions in `run-script` and
`exec`.

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

4 files changed, 357 insertions, 153 deletions

diff --git a/test/lib/docs.js b/test/lib/docs.js
index a7325738b..e8176eb47 100644
--- a/test/lib/docs.js
+++ b/test/lib/docs.js
@@ -1,51 +1,92 @@
 const t = require('tap')
-
 const requireInject = require('require-inject')
-const pacote = {
-  manifest: async (spec, options) => {
-    return spec === 'nodocs' ? {
+const mockNpm = require('../fixtures/mock-npm.js')
+const { join, sep } = require('path')
+
+const pkgDirs = t.testdir({
+  'package.json': JSON.stringify({
+    name: 'thispkg',
+    version: '1.2.3',
+    homepage: 'https://example.com',
+  }),
+  nodocs: {
+    'package.json': JSON.stringify({
       name: 'nodocs',
       version: '1.2.3',
-    }
-      : spec === 'docsurl' ? {
-        name: 'docsurl',
-        version: '1.2.3',
-        homepage: 'https://bugzilla.localhost/docsurl',
-      }
-      : spec === 'repourl' ? {
-        name: 'repourl',
-        version: '1.2.3',
-        repository: 'https://github.com/foo/repourl',
-      }
-      : spec === 'repoobj' ? {
-        name: 'repoobj',
-        version: '1.2.3',
-        repository: { url: 'https://github.com/foo/repoobj' },
-      }
-      : spec === '.' ? {
-        name: 'thispkg',
-        version: '1.2.3',
-        homepage: 'https://example.com',
-      }
-      : null
+    }),
   },
-}
+  docsurl: {
+    'package.json': JSON.stringify({
+      name: 'docsurl',
+      version: '1.2.3',
+      homepage: 'https://bugzilla.localhost/docsurl',
+    }),
+  },
+  repourl: {
+    'package.json': JSON.stringify({
+      name: 'repourl',
+      version: '1.2.3',
+      repository: 'https://github.com/foo/repourl',
+    }),
+  },
+  repoobj: {
+    'package.json': JSON.stringify({
+      name: 'repoobj',
+      version: '1.2.3',
+      repository: { url: 'https://github.com/foo/repoobj' },
+    }),
+  },
+  workspaces: {
+    'package.json': JSON.stringify({
+      name: 'workspaces-test',
+      version: '1.2.3-test',
+      workspaces: ['workspace-a', 'workspace-b', 'workspace-c'],
+    }),
+    'workspace-a': {
+      'package.json': JSON.stringify({
+        name: 'workspace-a',
+        version: '1.2.3-a',
+        homepage: 'http://docs.workspace-a/',
+      }),
+    },
+    'workspace-b': {
+      'package.json': JSON.stringify({
+        name: 'workspace-b',
+        version: '1.2.3-n',
+        repository: 'https://github.com/npm/workspace-b',
+      }),
+    },
+    'workspace-c': JSON.stringify({
+      'package.json': {
+        name: 'workspace-n',
+        version: '1.2.3-n',
+      },
+    }),
+  },
+})
 
 // keep a tally of which urls got opened
-const opened = {}
+let opened = {}
 const openUrl = async (npm, url, errMsg) => {
   opened[url] = opened[url] || 0
   opened[url]++
 }
 
 const Docs = requireInject('../../lib/docs.js', {
-  pacote,
   '../../lib/utils/open-url.js': openUrl,
 })
+const flatOptions = {}
+const npm = mockNpm({ flatOptions })
+const docs = new Docs(npm)
 
-const docs = new Docs({ flatOptions: {} })
+t.afterEach(async () => {
+  opened = {}
+})
 
 t.test('open docs urls', t => {
+  // XXX It is very odd that `where` is how pacote knows to look anywhere other
+  // than the cwd. I would think npm.localPrefix would factor in somehow
+  flatOptions.where = pkgDirs
   const expect = {
     nodocs: 'https://www.npmjs.com/package/nodocs',
     docsurl: 'https://bugzilla.localhost/docsurl',
@@ -57,11 +98,13 @@ t.test('open docs urls', t => {
   t.plan(keys.length)
   keys.forEach(pkg => {
     t.test(pkg, t => {
-      docs.exec([pkg], (er) => {
-        if (er)
-          throw er
+      docs.exec([['.', pkg].join(sep)], (err) => {
+        if (err)
+          throw err
         const url = expect[pkg]
-        t.equal(opened[url], 1, url, {opened})
+        t.match({
+          [url]: 1,
+        }, opened, `opened ${url}`, {opened})
         t.end()
       })
     })
@@ -72,7 +115,42 @@ t.test('open default package if none specified', t => {
   docs.exec([], (er) => {
     if (er)
       throw er
-    t.equal(opened['https://example.com'], 2, 'opened expected url', {opened})
+    t.equal(opened['https://example.com'], 1, 'opened expected url', {opened})
     t.end()
   })
 })
+
+t.test('workspaces', (t) => {
+  flatOptions.where = undefined
+  npm.localPrefix = join(pkgDirs, 'workspaces')
+  t.test('all workspaces', (t) => {
+    docs.execWorkspaces([], [], (err) => {
+      t.notOk(err)
+      t.match({
+        'http://docs.workspace-a/': 1,
+        'https://github.com/npm/workspace-b#readme': 1,
+      }, opened, 'opened two valid docs urls')
+      t.end()
+    })
+  })
+
+  t.test('one workspace', (t) => {
+    docs.execWorkspaces([], ['workspace-a'], (err) => {
+      t.notOk(err)
+      t.match({
+        'http://docs.workspace-a/': 1,
+      }, opened, 'opened one requested docs urls')
+      t.end()
+    })
+  })
+
+  t.test('invalid workspace', (t) => {
+    docs.execWorkspaces([], ['workspace-x'], (err) => {
+      t.match(err, /No workspaces found/)
+      t.match(err, /workspace-x/)
+      t.match({}, opened, 'opened no docs urls')
+      t.end()
+    })
+  })
+  t.end()
+})
diff --git a/test/lib/load-all-commands.js b/test/lib/load-all-commands.js
index d7eb2eae0..cb3aefb1f 100644
--- a/test/lib/load-all-commands.js
+++ b/test/lib/load-all-commands.js
@@ -1,8 +1,7 @@
-// Thanks to nyc not working properly with proxies this doesn't affect
-// coverage. but it does ensure that every command has a usage that renders,
-// contains its name, a description, and if it has completion it is a function.
-// That it renders also ensures that any params we've defined in our commands
-// work.
+// Our coverage mapping means that stuff like this doen't count for coverage.
+// It does ensure that every command has a usage that renders, contains its
+// name, a description, and if it has completion it is a function.  That it
+// renders also ensures that any params we've defined in our commands work.
 const requireInject = require('require-inject')
 const npm = requireInject('../../lib/npm.js')
 const t = require('tap')
diff --git a/test/lib/repo.js b/test/lib/repo.js
index 9c22bbaea..c05c02d80 100644
--- a/test/lib/repo.js
+++ b/test/lib/repo.js
@@ -1,125 +1,206 @@
 const t = require('tap')
-
 const requireInject = require('require-inject')
-const pacote = {
-  manifest: async (spec, options) => {
-    return spec === 'norepo' ? {
+const mockNpm = require('../fixtures/mock-npm.js')
+const { join, sep } = require('path')
+
+const pkgDirs = t.testdir({
+  'package.json': JSON.stringify({
+    name: 'thispkg',
+    version: '1.2.3',
+    repository: 'https://example.com/thispkg.git',
+  }),
+  norepo: {
+    'package.json': JSON.stringify({
       name: 'norepo',
       version: '1.2.3',
-    }
-
-      : spec === 'repoobbj-nourl' ? {
-        name: 'repoobj-nourl',
-        repository: { no: 'url' },
-      }
-
-      : spec === 'hostedgit' ? {
-        repository: 'git://github.com/foo/hostedgit',
-      }
-      : spec === 'hostedgitat' ? {
-        repository: 'git@github.com:foo/hostedgitat',
-      }
-      : spec === 'hostedssh' ? {
-        repository: 'ssh://git@github.com/foo/hostedssh',
-      }
-      : spec === 'hostedgitssh' ? {
-        repository: 'git+ssh://git@github.com/foo/hostedgitssh',
-      }
-      : spec === 'hostedgithttp' ? {
-        repository: 'git+http://github.com/foo/hostedgithttp',
-      }
-      : spec === 'hostedgithttps' ? {
-        repository: 'git+https://github.com/foo/hostedgithttps',
-      }
-
-      : spec === 'hostedgitobj' ? {
-        repository: { url: 'git://github.com/foo/hostedgitobj' },
-      }
-      : spec === 'hostedgitatobj' ? {
-        repository: { url: 'git@github.com:foo/hostedgitatobj' },
-      }
-      : spec === 'hostedsshobj' ? {
-        repository: { url: 'ssh://git@github.com/foo/hostedsshobj' },
-      }
-      : spec === 'hostedgitsshobj' ? {
-        repository: { url: 'git+ssh://git@github.com/foo/hostedgitsshobj' },
-      }
-      : spec === 'hostedgithttpobj' ? {
-        repository: { url: 'git+http://github.com/foo/hostedgithttpobj' },
-      }
-      : spec === 'hostedgithttpsobj' ? {
-        repository: { url: 'git+https://github.com/foo/hostedgithttpsobj' },
-      }
-
-      : spec === 'unhostedgit' ? {
-        repository: 'git://gothib.com/foo/unhostedgit',
-      }
-      : spec === 'unhostedgitat' ? {
-        repository: 'git@gothib.com:foo/unhostedgitat',
-      }
-      : spec === 'unhostedssh' ? {
-        repository: 'ssh://git@gothib.com/foo/unhostedssh',
-      }
-      : spec === 'unhostedgitssh' ? {
-        repository: 'git+ssh://git@gothib.com/foo/unhostedgitssh',
-      }
-      : spec === 'unhostedgithttp' ? {
-        repository: 'git+http://gothib.com/foo/unhostedgithttp',
-      }
-      : spec === 'unhostedgithttps' ? {
-        repository: 'git+https://gothib.com/foo/unhostedgithttps',
-      }
-
-      : spec === 'unhostedgitobj' ? {
-        repository: { url: 'git://gothib.com/foo/unhostedgitobj' },
-      }
-      : spec === 'unhostedgitatobj' ? {
-        repository: { url: 'git@gothib.com:foo/unhostedgitatobj' },
-      }
-      : spec === 'unhostedsshobj' ? {
-        repository: { url: 'ssh://git@gothib.com/foo/unhostedsshobj' },
-      }
-      : spec === 'unhostedgitsshobj' ? {
-        repository: { url: 'git+ssh://git@gothib.com/foo/unhostedgitsshobj' },
-      }
-      : spec === 'unhostedgithttpobj' ? {
-        repository: { url: 'git+http://gothib.com/foo/unhostedgithttpobj' },
-      }
-      : spec === 'unhostedgithttpsobj' ? {
-        repository: { url: 'git+https://gothib.com/foo/unhostedgithttpsobj' },
-      }
-
-      : spec === 'directory' ? {
-        repository: {
-          type: 'git',
-          url: 'git+https://github.com/foo/test-repo-with-directory.git',
-          directory: 'some/directory',
-        },
-      }
-
-      : spec === '.' ? {
-        name: 'thispkg',
-        version: '1.2.3',
-        repository: 'https://example.com/thispkg.git',
-      }
-      : null
+    }),
   },
-}
+  'repoobbj-nourl': {
+    'package.json': JSON.stringify({
+      name: 'repoobj-nourl',
+      repository: { no: 'url' },
+    }),
+  },
+  hostedgit: {
+    'package.json': JSON.stringify({
+      repository: 'git://github.com/foo/hostedgit',
+    }),
+  },
+  hostedgitat: {
+    'package.json': JSON.stringify({
+      repository: 'git@github.com:foo/hostedgitat',
+    }),
+  },
+  hostedssh: {
+    'package.json': JSON.stringify({
+      repository: 'ssh://git@github.com/foo/hostedssh',
+    }),
+  },
+  hostedgitssh: {
+    'package.json': JSON.stringify({
+      repository: 'git+ssh://git@github.com/foo/hostedgitssh',
+    }),
+  },
+  hostedgithttp: {
+    'package.json': JSON.stringify({
+      repository: 'git+http://github.com/foo/hostedgithttp',
+    }),
+  },
+  hostedgithttps: {
+    'package.json': JSON.stringify({
+      repository: 'git+https://github.com/foo/hostedgithttps',
+    }),
+  },
+  hostedgitobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'git://github.com/foo/hostedgitobj' },
+    }),
+  },
+  hostedgitatobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'git@github.com:foo/hostedgitatobj' },
+    }),
+  },
+  hostedsshobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'ssh://git@github.com/foo/hostedsshobj' },
+    }),
+  },
+  hostedgitsshobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'git+ssh://git@github.com/foo/hostedgitsshobj' },
+    }),
+  },
+  hostedgithttpobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'git+http://github.com/foo/hostedgithttpobj' },
+    }),
+  },
+  hostedgithttpsobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'git+https://github.com/foo/hostedgithttpsobj' },
+    }),
+  },
+  unhostedgit: {
+    'package.json': JSON.stringify({
+      repository: 'git://gothib.com/foo/unhostedgit',
+    }),
+  },
+  unhostedgitat: {
+    'package.json': JSON.stringify({
+      repository: 'git@gothib.com:foo/unhostedgitat',
+    }),
+  },
+  unhostedssh: {
+    'package.json': JSON.stringify({
+      repository: 'ssh://git@gothib.com/foo/unhostedssh',
+    }),
+  },
+  unhostedgitssh: {
+    'package.json': JSON.stringify({
+      repository: 'git+ssh://git@gothib.com/foo/unhostedgitssh',
+    }),
+  },
+  unhostedgithttp: {
+    'package.json': JSON.stringify({
+      repository: 'git+http://gothib.com/foo/unhostedgithttp',
+    }),
+  },
+  unhostedgithttps: {
+    'package.json': JSON.stringify({
+      repository: 'git+https://gothib.com/foo/unhostedgithttps',
+    }),
+  },
+  unhostedgitobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'git://gothib.com/foo/unhostedgitobj' },
+    }),
+  },
+  unhostedgitatobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'git@gothib.com:foo/unhostedgitatobj' },
+    }),
+  },
+  unhostedsshobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'ssh://git@gothib.com/foo/unhostedsshobj' },
+    }),
+  },
+  unhostedgitsshobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'git+ssh://git@gothib.com/foo/unhostedgitsshobj' },
+    }),
+  },
+  unhostedgithttpobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'git+http://gothib.com/foo/unhostedgithttpobj' },
+    }),
+  },
+  unhostedgithttpsobj: {
+    'package.json': JSON.stringify({
+      repository: { url: 'git+https://gothib.com/foo/unhostedgithttpsobj' },
+    }),
+  },
+  directory: {
+    'package.json': JSON.stringify({
+      repository: {
+        type: 'git',
+        url: 'git+https://github.com/foo/test-repo-with-directory.git',
+        directory: 'some/directory',
+      },
+    }),
+  },
+  workspaces: {
+    'package.json': JSON.stringify({
+      name: 'workspaces-test',
+      version: '1.2.3-test',
+      workspaces: ['workspace-a', 'workspace-b', 'workspace-c'],
+    }),
+    'workspace-a': {
+      'package.json': JSON.stringify({
+        name: 'workspace-a',
+        version: '1.2.3-a',
+        repository: 'http://repo.workspace-a/',
+      }),
+    },
+    'workspace-b': {
+      'package.json': JSON.stringify({
+        name: 'workspace-b',
+        version: '1.2.3-n',
+        repository: 'https://github.com/npm/workspace-b',
+      }),
+    },
+    'workspace-c': JSON.stringify({
+      'package.json': {
+        name: 'workspace-n',
+        version: '1.2.3-n',
+      },
+    }),
+  },
+})
 
 // keep a tally of which urls got opened
-const opened = {}
+let opened = {}
 const openUrl = async (npm, url, errMsg) => {
   opened[url] = opened[url] || 0
   opened[url]++
 }
 
 const Repo = requireInject('../../lib/repo.js', {
-  pacote,
   '../../lib/utils/open-url.js': openUrl,
 })
-const repo = new Repo({ flatOptions: {} })
+const flatOptions = {}
+const npm = mockNpm({ flatOptions })
+const repo = new Repo(npm)
+
+t.afterEach(async () => {
+  opened = {}
+})
 
 t.test('open repo urls', t => {
+  // XXX It is very odd that `where` is how pacote knows to look anywhere other
+  // than the cwd. I would think npm.localPrefix would factor in somehow
+  flatOptions.where = pkgDirs
   const expect = {
     hostedgit: 'https://github.com/foo/hostedgit',
     hostedgitat: 'https://github.com/foo/hostedgitat',
@@ -150,11 +231,13 @@ t.test('open repo urls', t => {
   t.plan(keys.length)
   keys.forEach(pkg => {
     t.test(pkg, t => {
-      repo.exec([pkg], (er) => {
-        if (er)
-          throw er
+      repo.exec([['.', pkg].join(sep)], (err) => {
+        if (err)
+          throw err
         const url = expect[pkg]
-        t.equal(opened[url], 1, url, {opened})
+        t.match({
+          [url]: 1,
+        }, opened, `opened ${url}`, {opened})
         t.end()
       })
     })
@@ -162,6 +245,7 @@ t.test('open repo urls', t => {
 })
 
 t.test('fail if cannot figure out repo url', t => {
+  flatOptions.where = pkgDirs
   const cases = [
     'norepo',
     'repoobbj-nourl',
@@ -173,8 +257,8 @@ t.test('fail if cannot figure out repo url', t => {
 
   cases.forEach(pkg => {
     t.test(pkg, t => {
-      repo.exec([pkg], er => {
-        t.match(er, { pkgid: pkg })
+      repo.exec([['.', pkg].join(sep)], (err) => {
+        t.match(err, { pkgid: pkg })
         t.end()
       })
     })
@@ -182,10 +266,47 @@ t.test('fail if cannot figure out repo url', t => {
 })
 
 t.test('open default package if none specified', t => {
+  flatOptions.where = pkgDirs
   repo.exec([], (er) => {
     if (er)
       throw er
-    t.equal(opened['https://example.com/thispkg'], 2, 'opened expected url', {opened})
+    t.equal(opened['https://example.com/thispkg'], 1, 'opened expected url', {opened})
     t.end()
   })
 })
+
+t.test('workspaces', t => {
+  flatOptions.where = undefined
+  npm.localPrefix = join(pkgDirs, 'workspaces')
+
+  t.test('all workspaces', (t) => {
+    repo.execWorkspaces([], [], (err) => {
+      t.notOk(err)
+      t.match({
+        'https://repo.workspace-a/': 1, // Gets translated to https!
+        'https://github.com/npm/workspace-b': 1,
+      }, opened, 'opened two valid repo urls')
+      t.end()
+    })
+  })
+
+  t.test('one workspace', (t) => {
+    repo.execWorkspaces([], ['workspace-a'], (err) => {
+      t.notOk(err)
+      t.match({
+        'https://repo.workspace-a/': 1,
+      }, opened, 'opened one requested repo urls')
+      t.end()
+    })
+  })
+
+  t.test('invalid workspace', (t) => {
+    repo.execWorkspaces([], ['workspace-x'], (err) => {
+      t.match(err, /No workspaces found/)
+      t.match(err, /workspace-x/)
+      t.match({}, opened, 'opened no repo urls')
+      t.end()
+    })
+  })
+  t.end()
+})
diff --git a/test/lib/utils/config/definition.js b/test/lib/utils/config/definition.js
index 56e10da0c..88a527db6 100644
--- a/test/lib/utils/config/definition.js
+++ b/test/lib/utils/config/definition.js
@@ -22,7 +22,7 @@ t.test('basic definition', async t => {
     defaultDescription: '"some default value"',
     type: [Number, String],
     hint: '<key>',
-    usage: '--key <key>|--key <key>',
+    usage: '--key <key>',
     typeDescription: 'Number or String',
     description: 'just a test thingie',
   })
@@ -113,6 +113,12 @@ t.test('basic definition', async t => {
     hint: '<testparam>',
   })
   t.equal(hasHint.usage, '--key <testparam>')
+  const optionalBool = new Definition('key', {
+    default: null,
+    type: [null, Boolean],
+    description: 'asdf',
+  })
+  t.equal(optionalBool.usage, '--key')
 })
 
 t.test('missing fields', async t => {