Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(ci): pay attention to --ignore-scripts
Added a test for install too for this specific condition

PR-URL: #2455
Credit: @wraithgar
Close: #2455
Reviewed-by: @isaacs
  • Loading branch information
wraithgar authored and isaacs committed Jan 7, 2021
1 parent 99156df commit 907b34b
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 20 deletions.
40 changes: 21 additions & 19 deletions lib/ci.js
Expand Up @@ -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([
Expand 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)
}
Expand Down
35 changes: 35 additions & 0 deletions test/lib/ci.js
Expand Up @@ -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',
Expand Down
36 changes: 35 additions & 1 deletion test/lib/install.js
Expand Up @@ -72,14 +72,48 @@ 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 () => {},
'../../lib/npm.js': {
globalDir: 'path/to/node_modules/',
prefix: 'foo',
flatOptions: {
global: 'true',
global: true,
},
config: {
get: () => false,
Expand Down

0 comments on commit 907b34b

Please sign in to comment.