diff --git a/lib/utils/open-url-prompt.js b/lib/utils/open-url-prompt.js index 290040e5d14a..df0c9709c077 100644 --- a/lib/utils/open-url-prompt.js +++ b/lib/utils/open-url-prompt.js @@ -1,5 +1,5 @@ const readline = require('readline') -const opener = require('opener') +const promiseSpawn = require('@npmcli/promise-spawn') function print (npm, title, url) { const json = npm.config.get('json') @@ -64,15 +64,7 @@ const promptOpen = async (npm, url, title, prompt, emitter) => { } const command = browser === true ? null : browser - await new Promise((resolve, reject) => { - opener(url, { command }, err => { - if (err) { - return reject(err) - } - - return resolve() - }) - }) + await promiseSpawn.open(url, { command }) } module.exports = promptOpen diff --git a/lib/utils/open-url.js b/lib/utils/open-url.js index eed2449dcce5..379640773fa6 100644 --- a/lib/utils/open-url.js +++ b/lib/utils/open-url.js @@ -1,4 +1,4 @@ -const opener = require('opener') +const promiseSpawn = require('@npmcli/promise-spawn') const { URL } = require('url') @@ -37,18 +37,14 @@ const open = async (npm, url, errMsg, isFile) => { } const command = browser === true ? null : browser - await new Promise((resolve, reject) => { - opener(url, { command }, (err) => { - if (err) { - if (err.code === 'ENOENT') { - printAlternateMsg() - } else { - return reject(err) - } + await promiseSpawn.open(url, { command }) + .catch((err) => { + if (err.code !== 'ENOENT') { + throw err } - return resolve() + + printAlternateMsg() }) - }) } module.exports = open diff --git a/smoke-tests/test/index.js b/smoke-tests/test/index.js index d9a2a8833884..7c196cb97eec 100644 --- a/smoke-tests/test/index.js +++ b/smoke-tests/test/index.js @@ -93,6 +93,7 @@ const exec = async (...args) => { env: { HOME: path, PATH: `${PATH}:${binLocation}`, + COMSPEC: process.env.COMSPEC, }, stdioString: true, encoding: 'utf-8', diff --git a/test/fixtures/sandbox.js b/test/fixtures/sandbox.js index b53c8f173a3a..c7bb8218dc60 100644 --- a/test/fixtures/sandbox.js +++ b/test/fixtures/sandbox.js @@ -166,14 +166,14 @@ class Sandbox extends EventEmitter { // replace default config values with placeholders for (const name of redactedDefaults) { const value = this[_npm].config.defaults[name] - clean = clean.split(value).join(`{${name.toUpperCase()}}`) + clean = clean.split(normalize(value)).join(`{${name.toUpperCase()}}`) } // replace vague default config values that are present within quotes // with placeholders for (const name of vagueRedactedDefaults) { const value = this[_npm].config.defaults[name] - clean = clean.split(`"${value}"`).join(`"{${name.toUpperCase()}}"`) + clean = clean.split(`"${normalize(value)}"`).join(`"{${name.toUpperCase()}}"`) } } diff --git a/test/lib/commands/edit.js b/test/lib/commands/edit.js index 22543f8e3cdc..dc7114892970 100644 --- a/test/lib/commands/edit.js +++ b/test/lib/commands/edit.js @@ -5,15 +5,11 @@ const { load: loadMockNpm } = require('../../fixtures/mock-npm') const spawk = tspawk(t) -// 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') - const npmConfig = { config: { 'ignore-scripts': false, editor: 'testeditor', + scriptShell: process.platform === 'win32' ? process.env.COMSPEC : 'sh', }, prefixDir: { node_modules: { @@ -34,30 +30,34 @@ const npmConfig = { }, } +const isCmdRe = /(?:^|\\)cmd(?:\.exe)?$/i + t.test('npm edit', async t => { const { npm, joinedOutput } = await loadMockNpm(t, npmConfig) const semverPath = path.resolve(npm.prefix, 'node_modules', 'semver') - const [scriptShell, scriptArgs] = makeSpawnArgs({ - event: 'install', - path: npm.prefix, - cmd: 'testinstall', - }) spawk.spawn('testeditor', [semverPath]) + + const scriptShell = npm.config.get('scriptShell') + const scriptArgs = isCmdRe.test(scriptShell) + ? ['/d', '/s', '/c', 'testinstall'] + : ['-c', 'testinstall'] spawk.spawn(scriptShell, scriptArgs, { cwd: semverPath }) + await npm.exec('edit', ['semver']) t.match(joinedOutput(), 'rebuilt dependencies successfully') }) t.test('rebuild failure', async t => { const { npm } = await loadMockNpm(t, npmConfig) + const semverPath = path.resolve(npm.prefix, 'node_modules', 'semver') - const [scriptShell, scriptArgs] = makeSpawnArgs({ - event: 'install', - path: npm.prefix, - cmd: 'testinstall', - }) spawk.spawn('testeditor', [semverPath]) + + const scriptShell = npm.config.get('scriptShell') + const scriptArgs = isCmdRe.test(scriptShell) + ? ['/d', '/s', '/c', 'testinstall'] + : ['-c', 'testinstall'] spawk.spawn(scriptShell, scriptArgs, { cwd: semverPath }).exit(1).stdout('test error') await t.rejects( npm.exec('edit', ['semver']), @@ -67,8 +67,10 @@ t.test('rebuild failure', async t => { t.test('editor failure', async t => { const { npm } = await loadMockNpm(t, npmConfig) + const semverPath = path.resolve(npm.prefix, 'node_modules', 'semver') spawk.spawn('testeditor', [semverPath]).exit(1).stdout('test editor failure') + await t.rejects( npm.exec('edit', ['semver']), { message: 'editor process exited with code: 1' } @@ -85,13 +87,14 @@ t.test('npm edit editor has flags', async t => { }) const semverPath = path.resolve(npm.prefix, 'node_modules', 'semver') - const [scriptShell, scriptArgs] = makeSpawnArgs({ - event: 'install', - path: npm.prefix, - cmd: 'testinstall', - }) spawk.spawn('testeditor', ['--flag', semverPath]) + + const scriptShell = npm.config.get('scriptShell') + const scriptArgs = isCmdRe.test(scriptShell) + ? ['/d', '/s', '/c', 'testinstall'] + : ['-c', 'testinstall'] spawk.spawn(scriptShell, scriptArgs, { cwd: semverPath }) + await npm.exec('edit', ['semver']) }) diff --git a/test/lib/commands/restart.js b/test/lib/commands/restart.js index c2257b8cebf8..f9745acdd11b 100644 --- a/test/lib/commands/restart.js +++ b/test/lib/commands/restart.js @@ -4,10 +4,7 @@ const { load: loadMockNpm } = require('../../fixtures/mock-npm') const spawk = tspawk(t) -// 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') +const isCmdRe = /(?:^|\\)cmd(?:\.exe)?$/i t.test('should run restart script from package.json', async t => { const { npm } = await loadMockNpm(t, { @@ -22,15 +19,14 @@ t.test('should run restart script from package.json', async t => { }, config: { loglevel: 'silent', + scriptShell: process.platform === 'win32' ? process.env.COMSPEC : 'sh', }, }) - const [scriptShell, scriptArgs] = makeSpawnArgs({ - path: npm.prefix, - cmd: 'node ./test-restart.js', - }) - let scriptContent = scriptArgs.pop() - scriptContent += ' foo' - scriptArgs.push(scriptContent) + + const scriptShell = npm.config.get('scriptShell') + const scriptArgs = isCmdRe.test(scriptShell) + ? ['/d', '/s', '/c', 'node ./test-restart.js foo'] + : ['-c', 'node ./test-restart.js foo'] const script = spawk.spawn(scriptShell, scriptArgs) await npm.exec('restart', ['foo']) t.ok(script.called, 'script ran') diff --git a/test/lib/commands/start.js b/test/lib/commands/start.js index 3caaa478f02e..47f7f1a6e0f5 100644 --- a/test/lib/commands/start.js +++ b/test/lib/commands/start.js @@ -4,10 +4,7 @@ const { load: loadMockNpm } = require('../../fixtures/mock-npm') const spawk = tspawk(t) -// 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') +const isCmdRe = /(?:^|\\)cmd(?:\.exe)?$/i t.test('should run start script from package.json', async t => { const { npm } = await loadMockNpm(t, { @@ -22,13 +19,14 @@ t.test('should run start script from package.json', async t => { }, config: { loglevel: 'silent', + scriptShell: process.platform === 'win32' ? process.env.COMSPEC : 'sh', }, }) - const [scriptShell, scriptArgs] = makeSpawnArgs({ path: npm.prefix, cmd: 'node ./test-start.js' }) - // we're calling the script with 'foo' as an argument, so add that to the script - let scriptContent = scriptArgs.pop() - scriptContent += ' foo' - scriptArgs.push(scriptContent) + + const scriptShell = npm.config.get('scriptShell') + const scriptArgs = isCmdRe.test(scriptShell) + ? ['/d', '/s', '/c', 'node ./test-start.js foo'] + : ['-c', 'node ./test-start.js foo'] const script = spawk.spawn(scriptShell, scriptArgs) await npm.exec('start', ['foo']) t.ok(script.called, 'script ran') diff --git a/test/lib/commands/stop.js b/test/lib/commands/stop.js index 72c2843adec9..9ca774288446 100644 --- a/test/lib/commands/stop.js +++ b/test/lib/commands/stop.js @@ -4,10 +4,7 @@ const { load: loadMockNpm } = require('../../fixtures/mock-npm') const spawk = tspawk(t) -// 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') +const isCmdRe = /(?:^|\\)cmd(?:\.exe)?$/i t.test('should run stop script from package.json', async t => { const { npm } = await loadMockNpm(t, { @@ -22,12 +19,14 @@ t.test('should run stop script from package.json', async t => { }, config: { loglevel: 'silent', + scriptShell: process.platform === 'win32' ? process.env.COMSPEC : 'sh', }, }) - const [scriptShell, scriptArgs] = makeSpawnArgs({ path: npm.prefix, cmd: 'node ./test-stop.js' }) - let scriptContent = scriptArgs.pop() - scriptContent += ' foo' - scriptArgs.push(scriptContent) + + const scriptShell = npm.config.get('scriptShell') + const scriptArgs = isCmdRe.test(scriptShell) + ? ['/d', '/s', '/c', 'node ./test-stop.js foo'] + : ['-c', 'node ./test-stop.js foo'] const script = spawk.spawn(scriptShell, scriptArgs) await npm.exec('stop', ['foo']) t.ok(script.called, 'script ran') diff --git a/test/lib/commands/test.js b/test/lib/commands/test.js index a6a6f723afe7..3a62b6a2d31b 100644 --- a/test/lib/commands/test.js +++ b/test/lib/commands/test.js @@ -4,10 +4,7 @@ const { load: loadMockNpm } = require('../../fixtures/mock-npm') const spawk = tspawk(t) -// 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') +const isCmdRe = /(?:^|\\)cmd(?:\.exe)?$/i t.test('should run test script from package.json', async t => { const { npm } = await loadMockNpm(t, { @@ -22,12 +19,14 @@ t.test('should run test script from package.json', async t => { }, config: { loglevel: 'silent', + scriptShell: process.platform === 'win32' ? process.env.COMSPEC : 'sh', }, }) - const [scriptShell, scriptArgs] = makeSpawnArgs({ path: npm.prefix, cmd: 'node ./test-test.js' }) - let scriptContent = scriptArgs.pop() - scriptContent += ' foo' - scriptArgs.push(scriptContent) + + const scriptShell = npm.config.get('scriptShell') + const scriptArgs = isCmdRe.test(scriptShell) + ? ['/d', '/s', '/c', 'node ./test-test.js foo'] + : ['-c', 'node ./test-test.js foo'] const script = spawk.spawn(scriptShell, scriptArgs) await npm.exec('test', ['foo']) t.ok(script.called, 'script ran') diff --git a/test/lib/utils/open-url-prompt.js b/test/lib/utils/open-url-prompt.js index 03f6b596d764..a18fe85f6875 100644 --- a/test/lib/utils/open-url-prompt.js +++ b/test/lib/utils/open-url-prompt.js @@ -21,10 +21,13 @@ const npm = { let openerUrl = null let openerOpts = null let openerResult = null -const opener = (url, opts, cb) => { + +const open = async (url, options) => { openerUrl = url - openerOpts = opts - return cb(openerResult) + openerOpts = options + if (openerResult) { + throw openerResult + } } let questionShouldResolve = true @@ -47,7 +50,9 @@ const readline = { } const openUrlPrompt = t.mock('../../../lib/utils/open-url-prompt.js', { - opener, + '@npmcli/promise-spawn': { + open, + }, readline, }) diff --git a/test/lib/utils/open-url.js b/test/lib/utils/open-url.js index 1bf47d8bbaed..70afd550333f 100644 --- a/test/lib/utils/open-url.js +++ b/test/lib/utils/open-url.js @@ -19,14 +19,19 @@ const npm = { let openerUrl = null let openerOpts = null let openerResult = null -const opener = (url, opts, cb) => { + +const open = async (url, options) => { openerUrl = url - openerOpts = opts - return cb(openerResult) + openerOpts = options + if (openerResult) { + throw openerResult + } } const openUrl = t.mock('../../../lib/utils/open-url.js', { - opener, + '@npmcli/promise-spawn': { + open, + }, }) t.test('opens a url', async t => {