Skip to content

Commit

Permalink
Improve cwd option
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky committed Feb 4, 2024
1 parent 97caf72 commit 7464b85
Show file tree
Hide file tree
Showing 8 changed files with 192 additions and 69 deletions.
23 changes: 8 additions & 15 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import {Buffer} from 'node:buffer';
import {setMaxListeners} from 'node:events';
import path from 'node:path';
import {basename} from 'node:path';
import childProcess from 'node:child_process';
import process from 'node:process';
import {fileURLToPath} from 'node:url';
import crossSpawn from 'cross-spawn';
import stripFinalNewline from 'strip-final-newline';
import {npmRunPathEnv} from 'npm-run-path';
Expand All @@ -16,6 +15,7 @@ import {getSpawnedResult, makeAllStream} from './lib/stream.js';
import {mergePromise} from './lib/promise.js';
import {joinCommand, getEscapedCommand} from './lib/escape.js';
import {parseCommand} from './lib/command.js';
import {getDefaultCwd, normalizeCwd, safeNormalizeFileUrl, normalizeFileUrl} from './lib/cwd.js';
import {parseTemplates} from './lib/script.js';
import {logCommand, verboseDefault} from './lib/verbose.js';
import {bufferToUint8Array} from './lib/stdio/utils.js';
Expand All @@ -32,17 +32,7 @@ const getEnv = ({env: envOption, extendEnv, preferLocal, localDir, execPath}) =>
return env;
};

const normalizeFileUrl = file => file instanceof URL ? fileURLToPath(file) : file;

const getFilePath = rawFile => {
const fileString = normalizeFileUrl(rawFile);

if (typeof fileString !== 'string') {
throw new TypeError('First argument must be a string or a file URL.');
}

return fileString;
};
const getFilePath = rawFile => safeNormalizeFileUrl(rawFile, 'First argument');

const handleArguments = (rawFile, rawArgs, rawOptions = {}) => {
const filePath = getFilePath(rawFile);
Expand All @@ -56,8 +46,9 @@ const handleArguments = (rawFile, rawArgs, rawOptions = {}) => {
options.shell = normalizeFileUrl(options.shell);
options.env = getEnv(options);
options.forceKillAfterDelay = normalizeForceKillAfterDelay(options.forceKillAfterDelay);
options.cwd = normalizeCwd(options.cwd);

if (process.platform === 'win32' && path.basename(file, '.exe') === 'cmd') {
if (process.platform === 'win32' && basename(file, '.exe') === 'cmd') {
// #116
args.unshift('/q');
}
Expand All @@ -73,7 +64,7 @@ const addDefaultOptions = ({
stripFinalNewline = true,
extendEnv = true,
preferLocal = false,
cwd = process.cwd(),
cwd = getDefaultCwd(),
localDir = cwd,
execPath = process.execPath,
encoding = 'utf8',
Expand Down Expand Up @@ -215,6 +206,7 @@ const handlePromise = async ({spawned, options, stdioStreamsGroups, originalStre
stdout: stdio[1],
stderr: stdio[2],
all,
cwd: options.cwd,
failed: false,
timedOut: false,
isCanceled: false,
Expand Down Expand Up @@ -281,6 +273,7 @@ export function execaSync(rawFile, rawArgs, rawOptions) {
stdio,
stdout: stdio[1],
stderr: stdio[2],
cwd: options.cwd,
failed: false,
timedOut: false,
isCanceled: false,
Expand Down
49 changes: 49 additions & 0 deletions lib/cwd.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import {statSync} from 'node:fs';
import {resolve} from 'node:path';
import {fileURLToPath} from 'node:url';
import process from 'node:process';

export const getDefaultCwd = () => {
try {
return process.cwd();
} catch (error) {
error.message = `The current directory does not exist.\n${error.message}`;
throw error;
}
};

export const normalizeCwd = cwd => {
const cwdString = safeNormalizeFileUrl(cwd, 'The "cwd" option');
return resolve(cwdString);
};

export const safeNormalizeFileUrl = (file, name) => {
const fileString = normalizeFileUrl(file);

if (typeof fileString !== 'string') {
throw new TypeError(`${name} must be a string or a file URL: ${fileString}.`);
}

return fileString;
};

export const normalizeFileUrl = file => file instanceof URL ? fileURLToPath(file) : file;

export const fixCwdError = (originalMessage, cwd) => {
if (cwd === getDefaultCwd()) {
return originalMessage;
}

let cwdStat;
try {
cwdStat = statSync(cwd);
} catch (error) {
return `The "cwd" option is invalid: ${cwd}.\n${error.message}\n${originalMessage}`;
}

if (!cwdStat.isDirectory()) {
return `The "cwd" option is not a directory: ${cwd}.\n${originalMessage}`;
}

return originalMessage;
};
7 changes: 4 additions & 3 deletions lib/error.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import process from 'node:process';
import {signalsByName} from 'human-signals';
import stripFinalNewline from 'strip-final-newline';
import {isBinary, binaryToString} from './stdio/utils.js';
import {fixCwdError} from './cwd.js';

const getErrorPrefix = ({timedOut, timeout, errorCode, signal, signalDescription, exitCode, isCanceled}) => {
if (timedOut) {
Expand Down Expand Up @@ -53,7 +53,7 @@ export const makeError = ({
escapedCommand,
timedOut,
isCanceled,
options: {timeoutDuration: timeout, cwd = process.cwd()},
options: {timeoutDuration: timeout, cwd},
}) => {
// `signal` and `exitCode` emitted on `spawned.on('exit')` event can be `null`.
// We normalize them to `undefined`
Expand All @@ -64,7 +64,8 @@ export const makeError = ({
const errorCode = error?.code;
const prefix = getErrorPrefix({timedOut, timeout, errorCode, signal, signalDescription, exitCode, isCanceled});
const execaMessage = `Command ${prefix}: ${command}`;
const originalMessage = previousErrors.has(error) ? error.originalMessage : String(error?.message ?? error);
const originalErrorMessage = previousErrors.has(error) ? error.originalMessage : String(error?.message ?? error);
const originalMessage = fixCwdError(originalErrorMessage, cwd);
const shortMessage = error === undefined ? execaMessage : `${execaMessage}\n${originalMessage}`;
const messageStdio = all === undefined ? [stdio[2], stdio[1]] : [all];
const message = [shortMessage, ...messageStdio, ...stdio.slice(3)]
Expand Down
103 changes: 103 additions & 0 deletions test/cwd.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import {mkdir, rmdir} from 'node:fs/promises';
import {relative, toNamespacedPath} from 'node:path';
import process from 'node:process';
import {pathToFileURL, fileURLToPath} from 'node:url';
import tempfile from 'tempfile';
import test from 'ava';
import {execa, execaSync} from '../index.js';
import {FIXTURES_DIR, setFixtureDir} from './helpers/fixtures-dir.js';

setFixtureDir();

const isWindows = process.platform === 'win32';

const testOptionCwdString = async (t, execaMethod) => {
const cwd = '/';
const {stdout} = await execaMethod('node', ['-p', 'process.cwd()'], {cwd});
t.is(toNamespacedPath(stdout), toNamespacedPath(cwd));
};

test('The "cwd" option can be a string', testOptionCwdString, execa);
test('The "cwd" option can be a string - sync', testOptionCwdString, execaSync);

const testOptionCwdUrl = async (t, execaMethod) => {
const cwd = '/';
const cwdUrl = pathToFileURL(cwd);
const {stdout} = await execaMethod('node', ['-p', 'process.cwd()'], {cwd: cwdUrl});
t.is(toNamespacedPath(stdout), toNamespacedPath(cwd));
};

test('The "cwd" option can be a URL', testOptionCwdUrl, execa);
test('The "cwd" option can be a URL - sync', testOptionCwdUrl, execaSync);

const testOptionCwdInvalid = (t, execaMethod) => {
t.throws(() => {
execaMethod('empty.js', {cwd: true});
}, {message: /The "cwd" option must be a string or a file URL: true/});
};

test('The "cwd" option cannot be an invalid type', testOptionCwdInvalid, execa);
test('The "cwd" option cannot be an invalid type - sync', testOptionCwdInvalid, execaSync);

const testErrorCwdDefault = async (t, execaMethod) => {
const {cwd} = await execaMethod('empty.js');
t.is(cwd, process.cwd());
};

test('The "cwd" option defaults to process.cwd()', testErrorCwdDefault, execa);
test('The "cwd" option defaults to process.cwd() - sync', testErrorCwdDefault, execaSync);

// Windows does not allow removing a directory used as `cwd` of a running process
if (!isWindows) {
const testCwdPreSpawn = async (t, execaMethod) => {
const currentCwd = process.cwd();
const filePath = tempfile();
await mkdir(filePath);
process.chdir(filePath);
await rmdir(filePath);

try {
t.throws(() => {
execaMethod('empty.js');
}, {message: /The current directory does not exist/});
} finally {
process.chdir(currentCwd);
}
};

test.serial('The "cwd" option default fails if current cwd is missing', testCwdPreSpawn, execa);
test.serial('The "cwd" option default fails if current cwd is missing - sync', testCwdPreSpawn, execaSync);
}

const cwdNotExisting = {cwd: 'does_not_exist', expectedCode: 'ENOENT', expectedMessage: 'The "cwd" option is invalid'};
const cwdTooLong = {cwd: '.'.repeat(1e5), expectedCode: 'ENAMETOOLONG', expectedMessage: 'The "cwd" option is invalid'};
const cwdNotDir = {cwd: fileURLToPath(import.meta.url), expectedCode: isWindows ? 'ENOENT' : 'ENOTDIR', expectedMessage: 'The "cwd" option is not a directory'};

const testCwdPostSpawn = async (t, {cwd, expectedCode, expectedMessage}, execaMethod) => {
const {failed, code, message} = await execaMethod('empty.js', {cwd, reject: false});
t.true(failed);
t.is(code, expectedCode);
t.true(message.includes(expectedMessage));
t.true(message.includes(cwd));
};

test('The "cwd" option must be an existing file', testCwdPostSpawn, cwdNotExisting, execa);
test('The "cwd" option must be an existing file - sync', testCwdPostSpawn, cwdNotExisting, execaSync);
test('The "cwd" option must not be too long', testCwdPostSpawn, cwdTooLong, execa);
test('The "cwd" option must not be too long - sync', testCwdPostSpawn, cwdTooLong, execaSync);
test('The "cwd" option must be a directory', testCwdPostSpawn, cwdNotDir, execa);
test('The "cwd" option must be a directory - sync', testCwdPostSpawn, cwdNotDir, execaSync);

const successProperties = {fixtureName: 'empty.js', expectedFailed: false};
const errorProperties = {fixtureName: 'fail.js', expectedFailed: true};

const testErrorCwd = async (t, execaMethod, {fixtureName, expectedFailed}) => {
const {failed, cwd} = await execaMethod(fixtureName, {cwd: relative('.', FIXTURES_DIR), reject: false});
t.is(failed, expectedFailed);
t.is(cwd, FIXTURES_DIR);
};

test('result.cwd is defined', testErrorCwd, execa, successProperties);
test('result.cwd is defined - sync', testErrorCwd, execaSync, successProperties);
test('error.cwd is defined', testErrorCwd, execa, errorProperties);
test('error.cwd is defined - sync', testErrorCwd, execaSync, errorProperties);
21 changes: 5 additions & 16 deletions test/error.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import process from 'node:process';
import test from 'ava';
import {execa, execaSync} from '../index.js';
import {FIXTURES_DIR, setFixtureDir} from './helpers/fixtures-dir.js';
import {setFixtureDir} from './helpers/fixtures-dir.js';
import {fullStdio, getStdio} from './helpers/stdio.js';
import {noopGenerator, outputObjectGenerator} from './helpers/generator.js';
import {foobarString} from './helpers/input.js';
Expand Down Expand Up @@ -144,8 +144,8 @@ test('error.message newlines are consistent - no newline', testErrorMessageConsi
test('error.message newlines are consistent - newline', testErrorMessageConsistent, 'stdout\n');

test('Original error.message is kept', async t => {
const {originalMessage} = await t.throwsAsync(execa('noop.js', {cwd: 1}));
t.true(originalMessage.startsWith('The "options.cwd" property must be of type string or an instance of Buffer or URL. Received type number'));
const {originalMessage} = await t.throwsAsync(execa('noop.js', {uid: true}));
t.is(originalMessage, 'The "options.uid" property must be int32. Received type boolean (true)');
});

test('failed is false on success', async t => {
Expand Down Expand Up @@ -279,21 +279,10 @@ test('error.code is undefined on success', async t => {
});

test('error.code is defined on failure if applicable', async t => {
const {code} = await t.throwsAsync(execa('noop.js', {cwd: 1}));
const {code} = await t.throwsAsync(execa('noop.js', {uid: true}));
t.is(code, 'ERR_INVALID_ARG_TYPE');
});

test('error.cwd is defined on failure if applicable', async t => {
const {cwd} = await t.throwsAsync(execa('fail.js', [], {cwd: FIXTURES_DIR}));
t.is(cwd, FIXTURES_DIR);
});

test('error.cwd is undefined on failure if not passed as options', async t => {
const expectedCwd = process.cwd();
const {cwd} = await t.throwsAsync(execa('fail.js'));
t.is(cwd, expectedCwd);
});

const testUnusualError = async (t, error) => {
const childProcess = execa('empty.js');
childProcess.emit('error', error);
Expand Down Expand Up @@ -326,7 +315,7 @@ test('error instance can be a plain object', async t => {
test('error instance can be shared', async t => {
const originalMessage = foobarString;
const error = new Error(originalMessage);
const fixtureName = 'noop.js';
const fixtureName = 'empty.js';

const firstArgument = 'one';
const childProcess = execa(fixtureName, [firstArgument]);
Expand Down
6 changes: 3 additions & 3 deletions test/helpers/fixtures-dir.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import path from 'node:path';
import {delimiter, resolve} from 'node:path';
import process from 'node:process';
import {fileURLToPath} from 'node:url';
import pathKey from 'path-key';

export const PATH_KEY = pathKey();
export const FIXTURES_DIR_URL = new URL('../fixtures/', import.meta.url);
export const FIXTURES_DIR = fileURLToPath(FIXTURES_DIR_URL);
export const FIXTURES_DIR = resolve(fileURLToPath(FIXTURES_DIR_URL));

// Add the fixtures directory to PATH so fixtures can be executed without adding
// `node`. This is only meant to make writing tests simpler.
export const setFixtureDir = () => {
process.env[PATH_KEY] = FIXTURES_DIR + path.delimiter + process.env[PATH_KEY];
process.env[PATH_KEY] = FIXTURES_DIR + delimiter + process.env[PATH_KEY];
};

10 changes: 5 additions & 5 deletions test/kill.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {setFixtureDir} from './helpers/fixtures-dir.js';

setFixtureDir();

const isWindows = process.platform === 'win32';
const TIMEOUT_REGEXP = /timed out after/;

const spawnNoKillable = async (forceKillAfterDelay, options) => {
Expand Down Expand Up @@ -47,7 +48,7 @@ test('`forceKillAfterDelay` should not be negative', testInvalidForceKill, -1);

// `SIGTERM` cannot be caught on Windows, and it always aborts the process (like `SIGKILL` on Unix).
// Therefore, this feature and those tests must be different on Windows.
if (process.platform === 'win32') {
if (isWindows) {
test('Can call `.kill()` with `forceKillAfterDelay` on Windows', async t => {
const {subprocess} = await spawnNoKillable(1);
subprocess.kill();
Expand Down Expand Up @@ -289,11 +290,10 @@ const pollForProcessExit = async pid => {
// - on Linux subprocesses are never killed regardless of `options.detached`
// With `options.cleanup`, subprocesses are always killed
// - `options.cleanup` with SIGKILL is a noop, since it cannot be handled
const exitIfWindows = process.platform === 'win32';
test('spawnAndKill SIGTERM', spawnAndKill, ['SIGTERM', false, false, exitIfWindows]);
test('spawnAndKill SIGKILL', spawnAndKill, ['SIGKILL', false, false, exitIfWindows]);
test('spawnAndKill SIGTERM', spawnAndKill, ['SIGTERM', false, false, isWindows]);
test('spawnAndKill SIGKILL', spawnAndKill, ['SIGKILL', false, false, isWindows]);
test('spawnAndKill cleanup SIGTERM', spawnAndKill, ['SIGTERM', true, false, true]);
test('spawnAndKill cleanup SIGKILL', spawnAndKill, ['SIGKILL', true, false, exitIfWindows]);
test('spawnAndKill cleanup SIGKILL', spawnAndKill, ['SIGKILL', true, false, isWindows]);
test('spawnAndKill detached SIGTERM', spawnAndKill, ['SIGTERM', false, true, false]);
test('spawnAndKill detached SIGKILL', spawnAndKill, ['SIGKILL', false, true, false]);
test('spawnAndKill cleanup detached SIGTERM', spawnAndKill, ['SIGTERM', true, true, false]);
Expand Down
Loading

0 comments on commit 7464b85

Please sign in to comment.