Skip to content

Commit

Permalink
prevent engines from running in the root package if devEngines is pre…
Browse files Browse the repository at this point in the history
…sent
  • Loading branch information
reggi committed Sep 13, 2024
1 parent 7613f03 commit 0094e7f
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 13 deletions.
50 changes: 50 additions & 0 deletions tap-snapshots/test/lib/commands/install.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,32 @@
* Make sure to inspect the output below. Do not ignore changes!
*/
'use strict'
exports[`test/lib/commands/install.js TAP devEngines should not utilize engines in root if devEngines is provided > must match snapshot 1`] = `
silly config load:file:{CWD}/npmrc
silly config load:file:{CWD}/prefix/.npmrc
silly config load:file:{CWD}/home/.npmrc
silly config load:file:{CWD}/global/etc/npmrc
verbose title npm
verbose argv "--fetch-retries" "0" "--cache" "{CWD}/cache" "--loglevel" "silly" "--color" "false"
verbose logfile logs-max:10 dir:{CWD}/cache/_logs/{DATE}-
verbose logfile {CWD}/cache/_logs/{DATE}-debug-0.log
silly logfile done cleaning log files
warn EBADDEVENGINES The developer of this package has specified the following through devEngines
warn EBADDEVENGINES Invalid engine "runtime"
warn EBADDEVENGINES Invalid semver version "0.0.1" does not match "v1337.0.0" for "runtime"
warn EBADDEVENGINES {
warn EBADDEVENGINES current: { name: 'node', version: 'v1337.0.0' },
warn EBADDEVENGINES required: { name: 'node', version: '0.0.1', onFail: 'warn' }
warn EBADDEVENGINES }
silly packumentCache heap:{heap} maxSize:{maxSize} maxEntrySize:{maxEntrySize}
silly idealTree buildDeps
silly reify moves {}
silly audit report null
up to date, audited 1 package in {TIME}
found 0 vulnerabilities
`

exports[`test/lib/commands/install.js TAP devEngines should show devEngines doesnt break engines > must match snapshot 1`] = `
silly config load:file:{CWD}/npmrc
silly config load:file:{CWD}/home/.npmrc
Expand Down Expand Up @@ -239,3 +265,27 @@ silly audit report null
up to date, audited 1 package in {TIME}
found 0 vulnerabilities
`

exports[`test/lib/commands/install.js TAP devEngines should utilize engines in root if devEngines is not provided > must match snapshot 1`] = `
silly config load:file:{CWD}/npmrc
silly config load:file:{CWD}/prefix/.npmrc
silly config load:file:{CWD}/home/.npmrc
silly config load:file:{CWD}/global/etc/npmrc
verbose title npm
verbose argv "--fetch-retries" "0" "--cache" "{CWD}/cache" "--loglevel" "silly" "--color" "false"
verbose logfile logs-max:10 dir:{CWD}/cache/_logs/{DATE}-
verbose logfile {CWD}/cache/_logs/{DATE}-debug-0.log
silly logfile done cleaning log files
silly packumentCache heap:{heap} maxSize:{maxSize} maxEntrySize:{maxEntrySize}
silly idealTree buildDeps
warn EBADENGINE Unsupported engine {
warn EBADENGINE package: undefined,
warn EBADENGINE required: { node: '0.0.1' },
warn EBADENGINE current: { node: 'v1337.0.0', npm: '42.0.0' }
warn EBADENGINE }
silly reify moves {}
silly audit report null
up to date, audited 1 package in {TIME}
found 0 vulnerabilities
`
89 changes: 77 additions & 12 deletions test/lib/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,9 @@ t.test('devEngines', async t => {
},
})
await npm.exec('install', [])
t.matchSnapshot(joinedFullOutput())
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(!output.includes('EBADDEVENGINES'))
})

t.test('should utilize devEngines failure case', async t => {
Expand All @@ -458,7 +460,9 @@ t.test('devEngines', async t => {
await t.rejects(
npm.exec('install', [])
)
t.matchSnapshot(joinedFullOutput())
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(output.includes('error EBADDEVENGINES'))
})

t.test('should utilize devEngines failure force case', async t => {
Expand All @@ -480,7 +484,9 @@ t.test('devEngines', async t => {
},
})
await npm.exec('install', [])
t.matchSnapshot(joinedFullOutput())
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(output.includes('warn EBADDEVENGINES'))
})

t.test('should utilize devEngines 2x warning case', async t => {
Expand All @@ -504,7 +510,9 @@ t.test('devEngines', async t => {
},
})
await npm.exec('install', [])
t.matchSnapshot(joinedFullOutput())
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(output.includes('warn EBADDEVENGINES'))
})

t.test('should utilize devEngines 2x error case', async t => {
Expand All @@ -530,7 +538,9 @@ t.test('devEngines', async t => {
await t.rejects(
npm.exec('install', [])
)
t.matchSnapshot(joinedFullOutput())
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(output.includes('error EBADDEVENGINES'))
})

t.test('should utilize devEngines failure and warning case', async t => {
Expand All @@ -555,10 +565,12 @@ t.test('devEngines', async t => {
await t.rejects(
npm.exec('install', [])
)
t.matchSnapshot(joinedFullOutput())
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(output.includes('EBADDEVENGINES'))
})

await t.test('should show devEngines has no effect on package install', async t => {
t.test('should show devEngines has no effect on package install', async t => {
const { npm, joinedFullOutput } = await loadMockNpm(t, {
...mockArguments,
prefixDir: {
Expand All @@ -575,10 +587,12 @@ t.test('devEngines', async t => {
},
})
await npm.exec('install', ['./alpha'])
t.matchSnapshot(joinedFullOutput())
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(!output.includes('EBADDEVENGINES'))
})

await t.test('should show devEngines has no effect on dev package install', async t => {
t.test('should show devEngines has no effect on dev package install', async t => {
const { npm, joinedFullOutput } = await loadMockNpm(t, {
...mockArguments,
prefixDir: {
Expand All @@ -598,10 +612,12 @@ t.test('devEngines', async t => {
},
})
await npm.exec('install', ['./alpha'])
t.matchSnapshot(joinedFullOutput())
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(!output.includes('EBADDEVENGINES'))
})

await t.test('should show devEngines doesnt break engines', async t => {
t.test('should show devEngines doesnt break engines', async t => {
const { npm, joinedFullOutput } = await loadMockNpm(t, {
...mockArguments,
prefixDir: {
Expand All @@ -620,6 +636,55 @@ t.test('devEngines', async t => {
config: { global: true },
})
await npm.exec('install', ['./alpha'])
t.matchSnapshot(joinedFullOutput())
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(output.includes('warn EBADENGINE'))
})

t.test('should not utilize engines in root if devEngines is provided', async t => {
const { npm, joinedFullOutput } = await loadMockNpm(t, {
...mockArguments,
prefixDir: {
'package.json': JSON.stringify({
name: 'alpha',
engines: {
node: '0.0.1',
},
devEngines: {
runtime: {
name: 'node',
version: '0.0.1',
onFail: 'warn',
},
},
}),
'index.js': 'console.log("this is alpha index")',
},
})
await npm.exec('install')
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(!output.includes('EBADENGINE'))
t.ok(output.includes('warn EBADDEVENGINES'))
})

t.test('should utilize engines in root if devEngines is not provided', async t => {
const { npm, joinedFullOutput } = await loadMockNpm(t, {
...mockArguments,
prefixDir: {
'package.json': JSON.stringify({
name: 'alpha',
engines: {
node: '0.0.1',
},
}),
'index.js': 'console.log("this is alpha index")',
},
})
await npm.exec('install')
const output = joinedFullOutput()
t.matchSnapshot(output)
t.ok(output.includes('EBADENGINE'))
t.ok(!output.includes('EBADDEVENGINES'))
})
})
5 changes: 4 additions & 1 deletion workspaces/arborist/lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,10 @@ module.exports = cls => class IdealTreeBuilder extends cls {
for (const node of this.idealTree.inventory.values()) {
if (!node.optional) {
try {
checkEngine(node.package, npmVersion, nodeVersion, this.options.force)
// if devEngines is present in the root node we ignore the engines check
if (!(node.isRoot && node.package.devEngines)) {
checkEngine(node.package, npmVersion, nodeVersion, this.options.force)
}
} catch (err) {
if (engineStrict) {
throw err
Expand Down

0 comments on commit 0094e7f

Please sign in to comment.