fix(libnpmexec): don't detach output from npm

lib/exec.js

@@ -76,8 +76,8 @@ class Exec extends BaseCommand {
       localBin,
       log,
       globalBin,
-      output,
     } = this.npm
+    const output = (...outputArgs) => this.npm.output(...outputArgs)
     const scriptShell = this.npm.config.get('script-shell') || undefined
     const packages = this.npm.config.get('package')
     const yes = this.npm.config.get('yes')

lib/init.js

@@ -113,8 +113,13 @@ class Init extends BaseCommand {
       localBin,
       log,
       globalBin,
-      output,
     } = this.npm
+    // this function is definitely called.  But because of coverage map stuff
+    // it ends up both uncovered, and the coverage report doesn't even mention.
+    // the tests do assert that some output happens, so we know this line is
+    // being hit.
+    /* istanbul ignore next */
+    const output = (...outputArgs) => this.npm.output(...outputArgs)
     const locationMsg = await getLocationMsg({ color, path })
     const runPath = path
     const scriptShell = this.npm.config.get('script-shell') || undefined

tap-snapshots/test/lib/init.js.test.cjs

@@ -6,13 +6,28 @@
  */
 'use strict'
 exports[`test/lib/init.js TAP workspaces no args > should print helper info 1`] = `
-
+Array [
+  Array [
+    String(
+      This utility will walk you through creating a package.json file.
+      It only covers the most common items, and tries to guess sensible defaults.
+
+      See \`npm help init\` for definitive documentation on these fields
+      and exactly what they do.
+
+      Use \`npm install <pkg>\` afterwards to install a package and
+      save it as a dependency in the package.json file.
+
+      Press ^C at any time to quit.
+    ),
+  ],
+]
 `
 
 exports[`test/lib/init.js TAP workspaces no args, existing folder > should print helper info 1`] = `
-
+Array []
 `
 
 exports[`test/lib/init.js TAP workspaces with arg but missing workspace folder > should print helper info 1`] = `
-
+Array []
 `

test/fixtures/mock-npm.js

@@ -4,35 +4,54 @@
 
 const realConfig = require('../../lib/utils/config')
 
-const mockLog = {
-  clearProgress: () => {},
-  disableProgress: () => {},
-  enableProgress: () => {},
-  http: () => {},
-  info: () => {},
-  levels: [],
-  notice: () => {},
-  pause: () => {},
-  silly: () => {},
-  verbose: () => {},
-  warn: () => {},
-}
-const mockNpm = (base = {}) => {
-  const config = base.config || {}
-  const flatOptions = base.flatOptions || {}
-  return {
-    log: mockLog,
-    ...base,
-    flatOptions,
-    config: {
+class MockNpm {
+  constructor (base = {}) {
+    this._mockOutputs = []
+    this.isMockNpm = true
+    this.base = base
+
+    const config = base.config || {}
+
+    for (const attr in base) {
+      if (attr !== 'config') {
+        this[attr] = base[attr]
+      }
+    }
+
+    this.flatOptions = base.flatOptions || {}
+    this.config = {
       // for now just set `find` to what config.find should return
       // this works cause `find` is not an existing config entry
       find: (k) => ({...realConfig.defaults, ...config})[k],
       get: (k) => ({...realConfig.defaults, ...config})[k],
       set: (k, v) => config[k] = v,
       list: [{ ...realConfig.defaults, ...config}]
-    },
+    }
+    if (!this.log) {
+      this.log = {
+        clearProgress: () => {},
+        disableProgress: () => {},
+        enableProgress: () => {},
+        http: () => {},
+        info: () => {},
+        levels: [],
+        notice: () => {},
+        pause: () => {},
+        silly: () => {},
+        verbose: () => {},
+        warn: () => {},
+      }
+    }
+  }
+
+  output(...msg) {
+    if (this.base.output)
+      return this.base.output(msg)
+    this._mockOutputs.push(msg)
   }
 }
 
-module.exports = mockNpm
+// TODO export MockNpm, and change tests to use new MockNpm()
+module.exports = (base = {}) => {
+  return new MockNpm(base)
+}

test/lib/exec.js

@@ -1,8 +1,6 @@
 const t = require('tap')
 const mockNpm = require('../fixtures/mock-npm')
 const { resolve, delimiter } = require('path')
-const OUTPUT = []
-const output = (...msg) => OUTPUT.push(msg)
 
 const ARB_CTOR = []
 const ARB_ACTUAL_TREE = {}
@@ -36,6 +34,7 @@ const config = {
   package: [],
   'script-shell': 'shell-cmd',
 }
+
 const npm = mockNpm({
   flatOptions,
   config,
@@ -53,7 +52,6 @@ const npm = mockNpm({
       LOG_WARN.push(args)
     },
   },
-  output,
 })
 
 const RUN_SCRIPTS = []
@@ -225,7 +223,7 @@ t.test('npm exec <noargs>, run interactive shell', t => {
     ARB_CTOR.length = 0
     MKDIRPS.length = 0
     ARB_REIFY.length = 0
-    OUTPUT.length = 0
+    npm._mockOutputs.length = 0
     exec.exec([], er => {
       if (er)
         throw er
@@ -256,7 +254,7 @@ t.test('npm exec <noargs>, run interactive shell', t => {
     process.stdin.isTTY = true
     run(t, true, () => {
       t.strictSame(LOG_WARN, [])
-      t.strictSame(OUTPUT, [
+      t.strictSame(npm._mockOutputs, [
         [`\nEntering npm script environment at location:\n${process.cwd()}\nType 'exit' or ^D when finished\n`],
       ], 'printed message about interactive shell')
       t.end()
@@ -270,7 +268,7 @@ t.test('npm exec <noargs>, run interactive shell', t => {
 
     run(t, true, () => {
       t.strictSame(LOG_WARN, [])
-      t.strictSame(OUTPUT, [
+      t.strictSame(npm._mockOutputs, [
         [`\u001b[0m\u001b[0m\n\u001b[0mEntering npm script environment\u001b[0m\u001b[0m at location:\u001b[0m\n\u001b[0m\u001b[2m${process.cwd()}\u001b[22m\u001b[0m\u001b[1m\u001b[22m\n\u001b[1mType 'exit' or ^D when finished\u001b[22m\n\u001b[1m\u001b[22m`],
       ], 'printed message about interactive shell')
       t.end()
@@ -282,7 +280,7 @@ t.test('npm exec <noargs>, run interactive shell', t => {
     process.stdin.isTTY = false
     run(t, true, () => {
       t.strictSame(LOG_WARN, [])
-      t.strictSame(OUTPUT, [], 'no message about interactive shell')
+      t.strictSame(npm._mockOutputs, [], 'no message about interactive shell')
       t.end()
     })
   })
@@ -294,7 +292,7 @@ t.test('npm exec <noargs>, run interactive shell', t => {
       t.strictSame(LOG_WARN, [
         ['exec', 'Interactive mode disabled in CI environment'],
       ])
-      t.strictSame(OUTPUT, [], 'no message about interactive shell')
+      t.strictSame(npm._mockOutputs, [], 'no message about interactive shell')
       t.end()
     })
   })
@@ -316,7 +314,7 @@ t.test('npm exec <noargs>, run interactive shell', t => {
       ARB_CTOR.length = 0
       MKDIRPS.length = 0
       ARB_REIFY.length = 0
-      OUTPUT.length = 0
+      npm._mockOutputs.length = 0
       RUN_SCRIPTS.length = 0
       t.end()
     })
@@ -1195,22 +1193,22 @@ t.test('workspaces', t => {
           return rej(er)
 
         t.strictSame(LOG_WARN, [])
-        t.strictSame(OUTPUT, [
+        t.strictSame(npm._mockOutputs, [
           [`\nEntering npm script environment in workspace [email protected] at location:\n${resolve(npm.localPrefix, 'packages/a')}\nType 'exit' or ^D when finished\n`],
         ], 'printed message about interactive shell')
         res()
       })
     })
 
     config.color = true
-    OUTPUT.length = 0
+    npm._mockOutputs.length = 0
     await new Promise((res, rej) => {
       exec.execWorkspaces([], ['a'], er => {
         if (er)
           return rej(er)
 
         t.strictSame(LOG_WARN, [])
-        t.strictSame(OUTPUT, [
+        t.strictSame(npm._mockOutputs, [
           [`\u001b[0m\u001b[0m\n\u001b[0mEntering npm script environment\u001b[0m\u001b[0m in workspace \u001b[[email protected]\u001b[39m at location:\u001b[0m\n\u001b[0m\u001b[2m${resolve(npm.localPrefix, 'packages/a')}\u001b[22m\u001b[0m\u001b[1m\u001b[22m\n\u001b[1mType 'exit' or ^D when finished\u001b[22m\n\u001b[1m\u001b[22m`],
         ], 'printed message about interactive shell')
         res()

test/lib/init.js

@@ -3,7 +3,6 @@ const { resolve } = require('path')
 const t = require('tap')
 const mockNpm = require('../fixtures/mock-npm')
 
-let result = ''
 const npmLog = {
   disableProgress: () => null,
   enableProgress: () => null,
@@ -19,9 +18,6 @@ const config = {
 const npm = mockNpm({
   config,
   log: npmLog,
-  output: (...msg) => {
-    result += msg.join('\n')
-  },
 })
 const mocks = {
   '../../lib/utils/usage.js': () => 'usage instructions',
@@ -33,7 +29,6 @@ const _consolelog = console.log
 const noop = () => {}
 
 t.afterEach(() => {
-  result = ''
   config.yes = true
   config.package = undefined
   npm.log = npmLog
@@ -322,6 +317,9 @@ t.test('npm init error', t => {
 
 t.test('workspaces', t => {
   t.test('no args', t => {
+    t.teardown(() => {
+      npm._mockOutputs.length = 0
+    })
     npm.localPrefix = t.testdir({
       'package.json': JSON.stringify({
         name: 'top-level',
@@ -340,12 +338,15 @@ t.test('workspaces', t => {
       if (err)
         throw err
 
-      t.matchSnapshot(result, 'should print helper info')
+      t.matchSnapshot(npm._mockOutputs, 'should print helper info')
       t.end()
     })
   })
 
   t.test('no args, existing folder', t => {
+    t.teardown(() => {
+      npm._mockOutputs.length = 0
+    })
     // init-package-json prints directly to console.log
     // this avoids poluting test output with those logs
     console.log = noop
@@ -369,12 +370,15 @@ t.test('workspaces', t => {
       if (err)
         throw err
 
-      t.matchSnapshot(result, 'should print helper info')
+      t.matchSnapshot(npm._mockOutputs, 'should print helper info')
       t.end()
     })
   })
 
   t.test('with arg but missing workspace folder', t => {
+    t.teardown(() => {
+      npm._mockOutputs.length = 0
+    })
     // init-package-json prints directly to console.log
     // this avoids poluting test output with those logs
     console.log = noop
@@ -401,7 +405,7 @@ t.test('workspaces', t => {
       if (err)
         throw err
 
-      t.matchSnapshot(result, 'should print helper info')
+      t.matchSnapshot(npm._mockOutputs, 'should print helper info')
       t.end()
     })
   })