From 907b34b2ecc34ac376d989f824f7492064e43ef4 Mon Sep 17 00:00:00 2001 From: Michael Garvin Date: Thu, 7 Jan 2021 09:14:09 -0800 Subject: [PATCH] fix(ci): pay attention to --ignore-scripts Added a test for install too for this specific condition PR-URL: https://github.com/npm/cli/pull/2455 Credit: @wraithgar Close: #2455 Reviewed-by: @isaacs --- lib/ci.js | 40 +++++++++++++++++++++------------------- test/lib/ci.js | 35 +++++++++++++++++++++++++++++++++++ test/lib/install.js | 36 +++++++++++++++++++++++++++++++++++- 3 files changed, 91 insertions(+), 20 deletions(-) diff --git a/lib/ci.js b/lib/ci.js index 2917899c8255..89c6b8f42074 100644 --- a/lib/ci.js +++ b/lib/ci.js @@ -21,7 +21,7 @@ const ci = async () => { } const where = npm.prefix - const { scriptShell } = npm.flatOptions + const { scriptShell, ignoreScripts } = npm.flatOptions const arb = new Arborist({ ...npm.flatOptions, path: where }) await Promise.all([ @@ -39,24 +39,26 @@ const ci = async () => { await arb.reify({ ...npm.flatOptions, save: false }) // run the same set of scripts that `npm install` runs. - const scripts = [ - 'preinstall', - 'install', - 'postinstall', - 'prepublish', // XXX should we remove this finally?? - 'preprepare', - 'prepare', - 'postprepare', - ] - for (const event of scripts) { - await runScript({ - path: where, - args: [], - scriptShell, - stdio: 'inherit', - stdioString: true, - event, - }) + if (!ignoreScripts) { + const scripts = [ + 'preinstall', + 'install', + 'postinstall', + 'prepublish', // XXX should we remove this finally?? + 'preprepare', + 'prepare', + 'postprepare', + ] + for (const event of scripts) { + await runScript({ + path: where, + args: [], + scriptShell, + stdio: 'inherit', + stdioString: true, + event, + }) + } } await reifyFinish(arb) } diff --git a/test/lib/ci.js b/test/lib/ci.js index c32fb83279a4..c8d8e169e9b7 100644 --- a/test/lib/ci.js +++ b/test/lib/ci.js @@ -6,6 +6,41 @@ const { test } = require('tap') const requireInject = require('require-inject') +test('should ignore scripts with --ignore-scripts', (t) => { + const SCRIPTS = [] + let REIFY_CALLED = false + const ci = requireInject('../../lib/ci.js', { + '../../lib/utils/reify-finish.js': async () => {}, + '../../lib/npm.js': { + globalDir: 'path/to/node_modules/', + prefix: 'foo', + flatOptions: { + global: false, + ignoreScripts: true + }, + config: { + get: () => false, + }, + }, + '@npmcli/run-script': ({ event }) => { + SCRIPTS.push(event) + }, + '@npmcli/arborist': function () { + this.loadVirtual = async () => {} + this.reify = () => { + REIFY_CALLED = true + } + }, + }) + ci([], er => { + if (er) + throw er + t.equal(REIFY_CALLED, true, 'called reify') + t.strictSame(SCRIPTS, [], 'no scripts when running ci') + t.end() + }) +}) + test('should use Arborist and run-script', (t) => { const scripts = [ 'preinstall', diff --git a/test/lib/install.js b/test/lib/install.js index 7e243e7ff35f..d94c9b5f1477 100644 --- a/test/lib/install.js +++ b/test/lib/install.js @@ -72,6 +72,40 @@ test('should install using Arborist', (t) => { t.end() }) +test('should ignore scripts with --ignore-scripts', (t) => { + const SCRIPTS = [] + let REIFY_CALLED = false + const install = requireInject('../../lib/install.js', { + '../../lib/utils/reify-finish.js': async () => {}, + '../../lib/npm.js': { + globalDir: 'path/to/node_modules/', + prefix: 'foo', + flatOptions: { + global: false, + ignoreScripts: true + }, + config: { + get: () => false, + }, + }, + '@npmcli/run-script': ({ event }) => { + SCRIPTS.push(event) + }, + '@npmcli/arborist': function () { + this.reify = () => { + REIFY_CALLED = true + } + }, + }) + install([], er => { + if (er) + throw er + t.equal(REIFY_CALLED, true, 'called reify') + t.strictSame(SCRIPTS, [], 'no scripts when adding dep') + t.end() + }) +}) + test('should install globally using Arborist', (t) => { const install = requireInject('../../lib/install.js', { '../../lib/utils/reify-finish.js': async () => {}, @@ -79,7 +113,7 @@ test('should install globally using Arborist', (t) => { globalDir: 'path/to/node_modules/', prefix: 'foo', flatOptions: { - global: 'true', + global: true, }, config: { get: () => false,