Skip to content

Commit

Permalink
Do not run interactive exec in CI when a TTY
Browse files Browse the repository at this point in the history
Credit: @isaacs
PR-URL: #2202
Close: #2202
Reviewed-by: @darcyclarke
  • Loading branch information
isaacs authored and darcyclarke committed Dec 2, 2020
1 parent 15d7333 commit f864b7b
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 28 deletions.
4 changes: 2 additions & 2 deletions docs/content/commands/npm-exec.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ npx -c '<cmd> [args...]'
npx -p <pkg>[@<specifier>] -c '<cmd> [args...]'
Run without --call or positional args to open interactive subshell


alias: npm x, npx

common options:
Expand All @@ -35,7 +34,8 @@ as running it via `npm run`.

Run without positional arguments or `--call`, this allows you to
interactively run commands in the same sort of shell environment that
`package.json` scripts are run.
`package.json` scripts are run. Interactive mode is not supported in CI
environments when standard input is a TTY, to prevent hangs.

Whatever packages are specified by the `--package` option will be
provided in the `PATH` of the executed command, along with any locally
Expand Down
11 changes: 8 additions & 3 deletions lib/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ const run = async ({ args, call, pathArr, shell }) => {

npm.log.disableProgress()
try {
if (script === shell) {
if (process.stdin.isTTY) {
if (ciDetect())
return npm.log.warn('exec', 'Interactive mode disabled in CI environment')
output(`\nEntering npm script environment\nType 'exit' or ^D when finished\n`)
}
}
return await runScript({
...npm.flatOptions,
pkg,
Expand Down Expand Up @@ -111,7 +118,6 @@ const exec = async args => {

// nothing to maybe install, skip the arborist dance
if (!call && !args.length && !packages.length) {
output(`\nEntering npm script environment\nType 'exit' or ^D when finished\n`)
return await run({
args,
call,
Expand Down Expand Up @@ -194,13 +200,12 @@ const exec = async args => {

// no need to install if already present
if (add.length) {
const isTTY = process.stdin.isTTY && process.stdout.isTTY
if (!npm.flatOptions.yes) {
// set -n to always say no
if (npm.flatOptions.yes === false)
throw 'canceled'

if (!isTTY || ciDetect()) {
if (!process.stdin.isTTY || ciDetect()) {
npm.log.warn('exec', `The following package${
add.length === 1 ? ' was' : 's were'
} not found and will be installed: ${
Expand Down
85 changes: 62 additions & 23 deletions test/lib/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,30 +198,69 @@ t.test('npm exec foo, already present locally', async t => {
})

t.test('npm exec <noargs>, run interactive shell', async t => {
ARB_CTOR.length = 0
MKDIRPS.length = 0
ARB_REIFY.length = 0
OUTPUT.length = 0
await exec([], er => {
if (er)
throw er
CI_NAME = null
const { isTTY } = process.stdin
process.stdin.isTTY = true
t.teardown(() => process.stdin.isTTY = isTTY)

const run = async (t, doRun = true) => {
LOG_WARN.length = 0
ARB_CTOR.length = 0
MKDIRPS.length = 0
ARB_REIFY.length = 0
OUTPUT.length = 0
await exec([], er => {
if (er)
throw er
})
t.strictSame(MKDIRPS, [], 'no need to make any dirs')
t.strictSame(ARB_CTOR, [], 'no need to instantiate arborist')
t.strictSame(ARB_REIFY, [], 'no need to reify anything')
t.equal(PROGRESS_ENABLED, true, 'progress re-enabled')
if (doRun) {
t.match(RUN_SCRIPTS, [{
pkg: { scripts: { npx: 'shell-cmd' } },
banner: false,
path: process.cwd(),
stdioString: true,
event: 'npx',
env: { PATH: process.env.PATH },
stdio: 'inherit',
}])
} else
t.strictSame(RUN_SCRIPTS, [])
RUN_SCRIPTS.length = 0
}

t.test('print message when tty and not in CI', async t => {
CI_NAME = null
process.stdin.isTTY = true
await run(t)
t.strictSame(LOG_WARN, [])
t.strictSame(OUTPUT, [
['\nEntering npm script environment\nType \'exit\' or ^D when finished\n'],
], 'printed message about interactive shell')
})
t.strictSame(OUTPUT, [
['\nEntering npm script environment\nType \'exit\' or ^D when finished\n'],
], 'printed message about interactive shell')
t.strictSame(MKDIRPS, [], 'no need to make any dirs')
t.strictSame(ARB_CTOR, [], 'no need to instantiate arborist')
t.strictSame(ARB_REIFY, [], 'no need to reify anything')
t.equal(PROGRESS_ENABLED, true, 'progress re-enabled')
t.match(RUN_SCRIPTS, [{
pkg: { scripts: { npx: 'shell-cmd' } },
banner: false,
path: process.cwd(),
stdioString: true,
event: 'npx',
env: { PATH: process.env.PATH },
stdio: 'inherit',
}])

t.test('no message when not TTY', async t => {
CI_NAME = null
process.stdin.isTTY = false
await run(t)
t.strictSame(LOG_WARN, [])
t.strictSame(OUTPUT, [], 'no message about interactive shell')
})

t.test('print warning when in CI and interactive', async t => {
CI_NAME = 'travis-ci'
process.stdin.isTTY = true
await run(t, false)
t.strictSame(LOG_WARN, [
['exec', 'Interactive mode disabled in CI environment'],
])
t.strictSame(OUTPUT, [], 'no message about interactive shell')
})

t.end()
})

t.test('npm exec foo, not present locally or in central loc', async t => {
Expand Down

0 comments on commit f864b7b

Please sign in to comment.