From 8240e4b3aacb7947d319373a783c2212d9d5188f Mon Sep 17 00:00:00 2001 From: Winter Date: Sun, 2 Oct 2022 21:37:59 -0400 Subject: [PATCH 1/3] fix(libnpmpack): obey foregroundScripts --- DEPENDENCIES.md | 1 + workspaces/libnpmpack/lib/index.js | 6 ++- workspaces/libnpmpack/test/fixtures/tspawk.js | 15 ++++++ workspaces/libnpmpack/test/index.js | 47 +++++++++++++++++++ 4 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 workspaces/libnpmpack/test/fixtures/tspawk.js diff --git a/DEPENDENCIES.md b/DEPENDENCIES.md index a9db099d3962c..55717e4357a74 100644 --- a/DEPENDENCIES.md +++ b/DEPENDENCIES.md @@ -403,6 +403,7 @@ graph LR; libnpmpack-->npmcli-run-script["@npmcli/run-script"]; libnpmpack-->npmcli-template-oss["@npmcli/template-oss"]; libnpmpack-->pacote; + libnpmpack-->spawk; libnpmpack-->tap; libnpmpublish-->libnpmpack; libnpmpublish-->lodash.clonedeep; diff --git a/workspaces/libnpmpack/lib/index.js b/workspaces/libnpmpack/lib/index.js index 93428b37cb269..baccadc3354df 100644 --- a/workspaces/libnpmpack/lib/index.js +++ b/workspaces/libnpmpack/lib/index.js @@ -19,13 +19,15 @@ async function pack (spec = 'file:.', opts = {}) { // mode const banner = !opts.silent + const stdio = opts.foregroundScripts ? 'inherit' : 'pipe' + if (spec.type === 'directory') { // prepack await runScript({ ...opts, event: 'prepack', path: spec.fetchSpec, - stdio: 'inherit', + stdio, pkg: manifest, banner, }) @@ -52,7 +54,7 @@ async function pack (spec = 'file:.', opts = {}) { ...opts, event: 'postpack', path: spec.fetchSpec, - stdio: 'inherit', + stdio, pkg: manifest, banner, env: { diff --git a/workspaces/libnpmpack/test/fixtures/tspawk.js b/workspaces/libnpmpack/test/fixtures/tspawk.js new file mode 100644 index 0000000000000..6c1b481c8a8f9 --- /dev/null +++ b/workspaces/libnpmpack/test/fixtures/tspawk.js @@ -0,0 +1,15 @@ +'use strict' + +const spawk = require('spawk') + +module.exports = tspawk + +function tspawk (t) { + t.teardown(function () { + spawk.unload() + }) + t.afterEach(function () { + spawk.done() + }) + return spawk +} diff --git a/workspaces/libnpmpack/test/index.js b/workspaces/libnpmpack/test/index.js index d9ec1d12a6628..acbdfd9477c1c 100644 --- a/workspaces/libnpmpack/test/index.js +++ b/workspaces/libnpmpack/test/index.js @@ -1,10 +1,15 @@ 'use strict' const t = require('tap') + +const tspawk = require('./fixtures/tspawk.js') +const spawk = tspawk(t) + const fs = require('fs') const path = require('path') const pack = require('../lib/index.js') const tnock = require('./fixtures/tnock.js') +const { load: loadMockNpm } = require('../../../test/fixtures/mock-npm.js') const OPTS = { registry: 'https://mock.reg/', @@ -12,6 +17,11 @@ const OPTS = { const REG = OPTS.registry +// 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') + t.test('packs from local directory', async t => { const testDir = t.testdir({ 'package.json': JSON.stringify({ @@ -128,3 +138,40 @@ t.test('packs from registry spec', async t => { const tarball = await pack(spec, { ...OPTS }) t.ok(tarball) }) + +t.test('runs scripts in foreground when foregroundScripts === true', async t => { + const { npm } = await loadMockNpm(t, {}) + + const testDir = t.testdir({ + 'package.json': JSON.stringify({ + name: 'my-cool-pkg', + version: '1.0.0', + scripts: { + prepack: 'touch prepack', + postpack: 'touch postpack', + }, + }, null, 2), + }) + + const cwd = process.cwd() + process.chdir(testDir) + + const [scriptShell, scriptArgs] = makeSpawnArgs({ + event: 'prepack', + path: npm.prefix, + cmd: 'touch prepack', + }) + + const prepack = spawk.spawn(scriptShell, scriptArgs) + + await pack('file:.', { + packDestination: testDir, + foregroundScripts: true, + }) + + t.ok(prepack.called) + + t.teardown(async () => { + process.chdir(cwd) + }) +}) From d6bda3fd2030a1a940601c23318d7df030f126d2 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 5 Oct 2022 10:39:39 -0700 Subject: [PATCH 2/3] chore: add spawk as a dev dependency --- package-lock.json | 4 +++- workspaces/libnpmpack/package.json | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index e0309554fb515..08130ff5dbbde 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10111,8 +10111,9 @@ }, "node_modules/spawk": { "version": "1.7.1", + "resolved": "https://registry.npmjs.org/spawk/-/spawk-1.7.1.tgz", + "integrity": "sha512-qkPqVdPp5ICEeSYKB/qCkwIBB0IWQuouEvYenQvpTq15fqSQgutpH453NjEImrpCWTwQwj2bQjGp8YGHapEiWw==", "dev": true, - "license": "MIT", "engines": { "node": ">=12.0.0" } @@ -14020,6 +14021,7 @@ "@npmcli/eslint-config": "^3.1.0", "@npmcli/template-oss": "4.4.5", "nock": "^13.0.7", + "spawk": "^1.7.1", "tap": "^16.0.1" }, "engines": { diff --git a/workspaces/libnpmpack/package.json b/workspaces/libnpmpack/package.json index 2e3b35f726e58..cc4fe03aa8e15 100644 --- a/workspaces/libnpmpack/package.json +++ b/workspaces/libnpmpack/package.json @@ -25,6 +25,7 @@ "@npmcli/eslint-config": "^3.1.0", "@npmcli/template-oss": "4.4.5", "nock": "^13.0.7", + "spawk": "^1.7.1", "tap": "^16.0.1" }, "repository": { From a12bb398fcb99f35fa811a7a9bec106b1cc6f84b Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 5 Oct 2022 10:51:56 -0700 Subject: [PATCH 3/3] chore: simplify foregroundScripts test --- workspaces/libnpmpack/test/fixtures/tspawk.js | 1 + workspaces/libnpmpack/test/index.js | 7 ++----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/workspaces/libnpmpack/test/fixtures/tspawk.js b/workspaces/libnpmpack/test/fixtures/tspawk.js index 6c1b481c8a8f9..554a9d402c32f 100644 --- a/workspaces/libnpmpack/test/fixtures/tspawk.js +++ b/workspaces/libnpmpack/test/fixtures/tspawk.js @@ -5,6 +5,7 @@ const spawk = require('spawk') module.exports = tspawk function tspawk (t) { + spawk.preventUnmatched() t.teardown(function () { spawk.unload() }) diff --git a/workspaces/libnpmpack/test/index.js b/workspaces/libnpmpack/test/index.js index acbdfd9477c1c..fb50e197bceaf 100644 --- a/workspaces/libnpmpack/test/index.js +++ b/workspaces/libnpmpack/test/index.js @@ -3,13 +3,11 @@ const t = require('tap') const tspawk = require('./fixtures/tspawk.js') -const spawk = tspawk(t) const fs = require('fs') const path = require('path') const pack = require('../lib/index.js') const tnock = require('./fixtures/tnock.js') -const { load: loadMockNpm } = require('../../../test/fixtures/mock-npm.js') const OPTS = { registry: 'https://mock.reg/', @@ -140,7 +138,7 @@ t.test('packs from registry spec', async t => { }) t.test('runs scripts in foreground when foregroundScripts === true', async t => { - const { npm } = await loadMockNpm(t, {}) + const spawk = tspawk(t) const testDir = t.testdir({ 'package.json': JSON.stringify({ @@ -148,7 +146,6 @@ t.test('runs scripts in foreground when foregroundScripts === true', async t => version: '1.0.0', scripts: { prepack: 'touch prepack', - postpack: 'touch postpack', }, }, null, 2), }) @@ -158,7 +155,7 @@ t.test('runs scripts in foreground when foregroundScripts === true', async t => const [scriptShell, scriptArgs] = makeSpawnArgs({ event: 'prepack', - path: npm.prefix, + path: testDir, cmd: 'touch prepack', })