fix: consolidate split-package-names

It was only ever used by `npm edit` so it's inline now.
Rewrote `npm edit` tests to be real

8 files changed, 126 insertions, 149 deletions

diff --git a/test/fixtures/tspawk.js b/test/fixtures/tspawk.js
new file mode 100644
index 000000000..554a9d402
--- /dev/null
+++ b/test/fixtures/tspawk.js
@@ -0,0 +1,16 @@
+'use strict'
+
+const spawk = require('spawk')
+
+module.exports = tspawk
+
+function tspawk (t) {
+  spawk.preventUnmatched()
+  t.teardown(function () {
+    spawk.unload()
+  })
+  t.afterEach(function () {
+    spawk.done()
+  })
+  return spawk
+}
diff --git a/test/lib/commands/config.js b/test/lib/commands/config.js
index 821713147..42df8b52d 100644
--- a/test/lib/commands/config.js
+++ b/test/lib/commands/config.js
@@ -1,10 +1,10 @@
 const { join } = require('path')
 const { promisify } = require('util')
 const fs = require('fs')
-const spawk = require('spawk')
+const tspawk = require('../../fixtures/tspawk')
 const t = require('tap')
 
-spawk.preventUnmatched()
+const spawk = tspawk(t)
 
 const readFile = promisify(fs.readFile)
 
@@ -369,10 +369,6 @@ t.test('config edit', async t => {
     '.npmrc': 'foo=bar\nbar=baz',
   })
 
-  t.teardown(() => {
-    spawk.clean()
-  })
-
   const EDITOR = 'vim'
   const editor = spawk.spawn(EDITOR).exit(0)
 
@@ -394,10 +390,6 @@ t.test('config edit', async t => {
 })
 
 t.test('config edit - editor exits non-0', async t => {
-  t.teardown(() => {
-    spawk.clean()
-  })
-
   const EDITOR = 'vim'
   const editor = spawk.spawn(EDITOR).exit(1)
 
diff --git a/test/lib/commands/edit.js b/test/lib/commands/edit.js
index 92754f282..1943e8c5f 100644
--- a/test/lib/commands/edit.js
+++ b/test/lib/commands/edit.js
@@ -1,138 +1,137 @@
 const t = require('tap')
-const { resolve } = require('path')
-const { EventEmitter } = require('events')
+const path = require('path')
+const tspawk = require('../../fixtures/tspawk')
+const { load: loadMockNpm } = require('../../fixtures/mock-npm')
 
-let editorBin = null
-let editorArgs = null
-let editorOpts = null
-let EDITOR_CODE = 0
-const childProcess = {
-  spawn: (bin, args, opts) => {
-    // save for assertions
-    editorBin = bin
-    editorArgs = args
-    editorOpts = opts
+const spawk = tspawk(t)
 
-    const editorEvents = new EventEmitter()
-    process.nextTick(() => {
-      editorEvents.emit('exit', EDITOR_CODE)
-    })
-    return editorEvents
-  },
-}
+// TODO this ... smells.  npm "script-shell" config mentions defaults but those
+// are handled by run-script, not npm.  So for now we have to tie tests to some
+// pretty specific internals of runScript
+const makeSpawnArgs = require('@npmcli/run-script/lib/make-spawn-args.js')
 
-let rebuildArgs = null
-let rebuildFail = null
-let EDITOR = 'vim'
-const npm = {
+const npmConfig = {
   config: {
-    get: () => EDITOR,
+    'ignore-scripts': false,
+    editor: 'testeditor',
   },
-  dir: resolve(__dirname, '../../../node_modules'),
-  exec: async (cmd, args) => {
-    rebuildArgs = args
-    if (rebuildFail) {
-      throw rebuildFail
-    }
+  prefixDir: {
+    node_modules: {
+      semver: {
+        'package.json': JSON.stringify({
+          scripts: {
+            install: 'testinstall',
+          },
+        }),
+        node_modules: {
+          abbrev: {},
+        },
+      },
+      '@npmcli': {
+        'scoped-package': {},
+      },
+    },
   },
 }
 
-const gracefulFs = require('graceful-fs')
-const Edit = t.mock('../../../lib/commands/edit.js', {
-  child_process: childProcess,
-  'graceful-fs': gracefulFs,
-})
-const edit = new Edit(npm)
-
 t.test('npm edit', async t => {
-  t.teardown(() => {
-    rebuildArgs = null
-    editorBin = null
-    editorArgs = null
-    editorOpts = null
-  })
+  const { npm, joinedOutput } = await loadMockNpm(t, npmConfig)
 
-  await edit.exec(['semver'])
-  const path = resolve(__dirname, '../../../node_modules/semver')
-  t.strictSame(editorBin, EDITOR, 'used the correct editor')
-  t.strictSame(editorArgs, [path], 'edited the correct directory')
-  t.strictSame(editorOpts, { stdio: 'inherit' }, 'passed the correct opts')
-  t.strictSame(rebuildArgs, [path], 'passed the correct path to rebuild')
+  const semverPath = path.resolve(npm.prefix, 'node_modules', 'semver')
+  const [scriptShell] = makeSpawnArgs({
+    event: 'install',
+    path: npm.prefix,
+  })
+  spawk.spawn('testeditor', [semverPath])
+  spawk.spawn(
+    scriptShell,
+    args => args.includes('testinstall'),
+    { cwd: semverPath }
+  )
+  await npm.exec('edit', ['semver'])
+  t.match(joinedOutput(), 'rebuilt dependencies successfully')
 })
 
-t.test('rebuild fails', async t => {
-  t.teardown(() => {
-    rebuildFail = null
-    rebuildArgs = null
-    editorBin = null
-    editorArgs = null
-    editorOpts = null
+t.test('rebuild failure', async t => {
+  const { npm } = await loadMockNpm(t, npmConfig)
+  const semverPath = path.resolve(npm.prefix, 'node_modules', 'semver')
+  const [scriptShell] = makeSpawnArgs({
+    event: 'install',
+    path: npm.prefix,
   })
+  spawk.spawn('testeditor', [semverPath])
+  spawk.spawn(
+    scriptShell,
+    args => args.includes('testinstall'),
+    { cwd: semverPath }
+  ).exit(1).stdout('test error')
+  await t.rejects(
+    npm.exec('edit', ['semver']),
+    { message: 'command failed' }
+  )
+})
 
-  rebuildFail = new Error('test error')
+t.test('editor failure', async t => {
+  const { npm } = await loadMockNpm(t, npmConfig)
+  const semverPath = path.resolve(npm.prefix, 'node_modules', 'semver')
+  spawk.spawn('testeditor', [semverPath]).exit(1).stdout('test editor failure')
   await t.rejects(
-    edit.exec(['semver']),
-    { message: 'test error' }
+    npm.exec('edit', ['semver']),
+    { message: 'editor process exited with code: 1' }
   )
-  const path = resolve(__dirname, '../../../node_modules/semver')
-  t.strictSame(editorBin, EDITOR, 'used the correct editor')
-  t.strictSame(editorArgs, [path], 'edited the correct directory')
-  t.strictSame(editorOpts, { stdio: 'inherit' }, 'passed the correct opts')
-  t.strictSame(rebuildArgs, [path], 'passed the correct path to rebuild')
 })
 
 t.test('npm edit editor has flags', async t => {
-  EDITOR = 'code -w'
-  t.teardown(() => {
-    rebuildArgs = null
-    editorBin = null
-    editorArgs = null
-    editorOpts = null
-    EDITOR = 'vim'
+  const { npm } = await loadMockNpm(t, {
+    ...npmConfig,
+    config: {
+      ...npmConfig.config,
+      editor: 'testeditor --flag',
+    },
   })
 
-  await edit.exec(['semver'])
-
-  const path = resolve(__dirname, '../../../node_modules/semver')
-  t.strictSame(editorBin, 'code', 'used the correct editor')
-  t.strictSame(editorArgs, ['-w', path], 'edited the correct directory, keeping flags')
-  t.strictSame(editorOpts, { stdio: 'inherit' }, 'passed the correct opts')
-  t.strictSame(rebuildArgs, [path], 'passed the correct path to rebuild')
+  const semverPath = path.resolve(npm.prefix, 'node_modules', 'semver')
+  const [scriptShell] = makeSpawnArgs({
+    event: 'install',
+    path: npm.prefix,
+  })
+  spawk.spawn('testeditor', ['--flag', semverPath])
+  spawk.spawn(
+    scriptShell,
+    args => args.includes('testinstall'),
+    { cwd: semverPath }
+  )
+  await npm.exec('edit', ['semver'])
 })
 
 t.test('npm edit no args', async t => {
+  const { npm } = await loadMockNpm(t)
   await t.rejects(
-    edit.exec([]),
-    /npm edit/,
+    npm.exec('edit', []),
+    { code: 'EUSAGE' },
     'throws usage error'
   )
 })
 
-t.test('npm edit lstat error propagates', async t => {
-  const _lstat = gracefulFs.lstat
-  gracefulFs.lstat = (dir, cb) => {
-    return cb(new Error('lstat failed'))
-  }
-  t.teardown(() => {
-    gracefulFs.lstat = _lstat
-  })
+t.test('npm edit nonexistent package', async t => {
+  const { npm } = await loadMockNpm(t, npmConfig)
 
   await t.rejects(
-    edit.exec(['semver']),
-    /lstat failed/,
-    'user received correct error'
+    npm.exec('edit', ['abbrev']),
+    /lstat/
   )
 })
 
-t.test('npm edit editor exit code error propagates', async t => {
-  EDITOR_CODE = 137
-  t.teardown(() => {
-    EDITOR_CODE = 0
-  })
+t.test('scoped package', async t => {
+  const { npm } = await loadMockNpm(t, npmConfig)
+  const scopedPath = path.resolve(npm.prefix, 'node_modules', '@npmcli', 'scoped-package')
+  spawk.spawn('testeditor', [scopedPath])
+  await npm.exec('edit', ['@npmcli/scoped-package'])
+})
 
-  await t.rejects(
-    edit.exec(['semver']),
-    /exited with code: 137/,
-    'user received correct error'
-  )
+t.test('subdependency', async t => {
+  const { npm } = await loadMockNpm(t, npmConfig)
+  const subdepPath = path.resolve(npm.prefix, 'node_modules', 'semver', 'node_modules', 'abbrev')
+  spawk.spawn('testeditor', [subdepPath])
+  await npm.exec('edit', ['semver/abbrev'])
 })
diff --git a/test/lib/commands/restart.js b/test/lib/commands/restart.js
index 83773eae9..84bd93d8c 100644
--- a/test/lib/commands/restart.js
+++ b/test/lib/commands/restart.js
@@ -1,11 +1,8 @@
 const t = require('tap')
-const spawk = require('spawk')
+const tspawk = require('../../fixtures/tspawk')
 const { load: loadMockNpm } = require('../../fixtures/mock-npm')
 
-spawk.preventUnmatched()
-t.teardown(() => {
-  spawk.unload()
-})
+const spawk = tspawk(t)
 
 // TODO this ... smells.  npm "script-shell" config mentions defaults but those
 // are handled by run-script, not npm.  So for now we have to tie tests to some
diff --git a/test/lib/commands/start.js b/test/lib/commands/start.js
index c9312c8e2..8fc73493d 100644
--- a/test/lib/commands/start.js
+++ b/test/lib/commands/start.js
@@ -1,11 +1,8 @@
 const t = require('tap')
-const spawk = require('spawk')
+const tspawk = require('../../fixtures/tspawk')
 const { load: loadMockNpm } = require('../../fixtures/mock-npm')
 
-spawk.preventUnmatched()
-t.teardown(() => {
-  spawk.unload()
-})
+const spawk = tspawk(t)
 
 // TODO this ... smells.  npm "script-shell" config mentions defaults but those
 // are handled by run-script, not npm.  So for now we have to tie tests to some
diff --git a/test/lib/commands/stop.js b/test/lib/commands/stop.js
index f5db4a047..f2aef2189 100644
--- a/test/lib/commands/stop.js
+++ b/test/lib/commands/stop.js
@@ -1,11 +1,8 @@
 const t = require('tap')
-const spawk = require('spawk')
+const tspawk = require('../../fixtures/tspawk')
 const { load: loadMockNpm } = require('../../fixtures/mock-npm')
 
-spawk.preventUnmatched()
-t.teardown(() => {
-  spawk.unload()
-})
+const spawk = tspawk(t)
 
 // TODO this ... smells.  npm "script-shell" config mentions defaults but those
 // are handled by run-script, not npm.  So for now we have to tie tests to some
diff --git a/test/lib/commands/test.js b/test/lib/commands/test.js
index 665df7148..e9ea0a3c8 100644
--- a/test/lib/commands/test.js
+++ b/test/lib/commands/test.js
@@ -1,11 +1,8 @@
 const t = require('tap')
-const spawk = require('spawk')
+const tspawk = require('../../fixtures/tspawk')
 const { load: loadMockNpm } = require('../../fixtures/mock-npm')
 
-spawk.preventUnmatched()
-t.teardown(() => {
-  spawk.unload()
-})
+const spawk = tspawk(t)
 
 // TODO this ... smells.  npm "script-shell" config mentions defaults but those
 // are handled by run-script, not npm.  So for now we have to tie tests to some
diff --git a/test/lib/utils/split-package-names.js b/test/lib/utils/split-package-names.js
deleted file mode 100644
index 5fe1e6cd8..000000000
--- a/test/lib/utils/split-package-names.js
+++ /dev/null
@@ -1,18 +0,0 @@
-'use strict'
-
-const t = require('tap')
-const splitPackageNames = require('../../../lib/utils/split-package-names.js')
-
-t.test('splitPackageNames', t => {
-  const assertions = [
-    ['semver', 'semver'],
-    ['read-pkg/semver', 'read-pkg/node_modules/semver'],
-    ['@npmcli/one/@npmcli/two', '@npmcli/one/node_modules/@npmcli/two'],
-    ['@npmcli/one/semver', '@npmcli/one/node_modules/semver'],
-  ]
-
-  for (const [input, expected] of assertions) {
-    t.equal(splitPackageNames(input), expected, `split ${input} correctly`)
-  }
-  t.end()
-})