Skip to content

Commit

Permalink
Add more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky committed Sep 1, 2024
1 parent 58d6a3c commit 0b1672d
Show file tree
Hide file tree
Showing 12 changed files with 101 additions and 65 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
node_modules
yarn.lock
!/test/fixtures/**/node_modules
17 changes: 7 additions & 10 deletions source/context.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
import process from 'node:process';
import {stripVTControlCharacters} from 'node:util';

export const getContext = (previous, rawFile, rawArguments) => {
export const getContext = (previous, raw) => {
const start = previous.start ?? process.hrtime.bigint();
const command = [previous.command, getCommand(rawFile, rawArguments)].filter(Boolean).join(' | ');
const command = [previous.command, getCommand(raw)].filter(Boolean).join(' | ');
return {start, command, state: {stdout: '', stderr: '', output: ''}};
};

const getCommand = (rawFile, rawArguments) => [rawFile, ...rawArguments]
.map(part => getCommandPart(part))
const getCommand = raw => raw
.map(part => getCommandPart(stripVTControlCharacters(part)))
.join(' ');

const getCommandPart = part => {
part = stripVTControlCharacters(part);
return /[^\w./-]/.test(part)
? `'${part.replaceAll('\'', '\'\\\'\'')}'`
: part;
};
const getCommandPart = part => /[^\w./-]/.test(part)
? `'${part.replaceAll('\'', '\'\\\'\'')}'`
: part;
2 changes: 1 addition & 1 deletion source/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export default function nanoSpawn(first, second = [], third = {}) {
const [rawFile, previous] = Array.isArray(first) ? first : [first, {}];
const [rawArguments, options] = Array.isArray(second) ? [second, third] : [[], second];

const context = getContext(previous, rawFile, rawArguments);
const context = getContext(previous, [rawFile, ...rawArguments]);
const spawnOptions = getOptions(options);
const nodeChildProcess = spawnSubprocess(rawFile, rawArguments, spawnOptions, context);
const resultPromise = getResult(nodeChildProcess, spawnOptions, context);
Expand Down
6 changes: 2 additions & 4 deletions source/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@ export const getOptions = ({
}) => {
const cwd = cwdOption instanceof URL ? fileURLToPath(cwdOption) : path.resolve(cwdOption);
const env = envOption === undefined ? undefined : {...process.env, ...envOption};
const [stdioOption, input] = stdio[0]?.string === undefined
? [stdio]
: [['pipe', ...stdio.slice(1)], stdio[0].string];
const input = stdio[0]?.string;
return {
...options,
stdio: stdioOption,
input,
stdio: input === undefined ? stdio : ['pipe', ...stdio.slice(1)],
env: preferLocal ? addLocalPath(env ?? process.env, cwd) : env,
cwd,
};
Expand Down
34 changes: 15 additions & 19 deletions source/result.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
import {once, on} from 'node:events';
import process from 'node:process';

export const getResult = async (nodeChildProcess, options, context) => {
export const getResult = async (nodeChildProcess, {input}, context) => {
const instance = await nodeChildProcess;
useInput(instance, options);
if (input !== undefined) {
instance.stdin.end(input);
}

const onClose = once(instance, 'close');

try {
await Promise.race([onClose, ...onStreamErrors(instance)]);
await Promise.race([
onClose,
...instance.stdio.filter(Boolean).map(stream => onStreamError(stream)),
]);
checkFailure(context, getErrorOutput(instance));
return getOutputs(context);
} catch (error) {
Expand All @@ -16,25 +22,15 @@ export const getResult = async (nodeChildProcess, options, context) => {
}
};

const useInput = (instance, {input}) => {
if (input !== undefined) {
instance.stdin.end(input);
}
};

const onStreamErrors = ({stdio}) => stdio.filter(Boolean).map(stream => onStreamError(stream));

const onStreamError = async stream => {
for await (const [error] of on(stream, 'error')) {
if (!IGNORED_CODES.has(error?.code)) {
// Ignore errors that are due to closing errors when the subprocesses exit normally, or due to piping
if (!['ERR_STREAM_PREMATURE_CLOSE', 'EPIPE'].includes(error?.code)) {
throw error;
}
}
};

// Ignore errors that are due to closing errors when the subprocesses exit normally, or due to piping
const IGNORED_CODES = new Set(['ERR_STREAM_PREMATURE_CLOSE', 'EPIPE']);

const checkFailure = ({command}, {exitCode, signalName}) => {
if (signalName !== undefined) {
throw new Error(`Command was terminated with ${signalName}: ${command}`);
Expand All @@ -57,7 +53,7 @@ const getErrorInstance = (error, {command}) => error?.message.startsWith('Comman

const getErrorOutput = ({exitCode, signalCode}) => ({
// `exitCode` can be a negative number (`errno`) when the `error` event is emitted on the `instance`
...(exitCode === null || exitCode < 1 ? {} : {exitCode}),
...(exitCode < 1 ? {} : {exitCode}),
...(signalCode === null ? {} : {signalName: signalCode}),
});

Expand All @@ -69,6 +65,6 @@ const getOutputs = ({state: {stdout, stderr, output}, command, start}) => ({
durationMs: Number(process.hrtime.bigint() - start) / 1e6,
});

const getOutput = input => input?.at(-1) === '\n'
? input.slice(0, input.at(-2) === '\r' ? -2 : -1)
: input;
const getOutput = output => output.at(-1) === '\n'
? output.slice(0, output.at(-2) === '\r' ? -2 : -1)
: output;
7 changes: 3 additions & 4 deletions source/spawn.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const spawnSubprocess = async (rawFile, rawArguments, options, context) =
// When running `node`, keep the current Node version and CLI flags.
// Not applied with file paths to `.../node` since those indicate a clear intent to use a specific Node version.
// Does not work with shebangs, but those don't work cross-platform anyway.
const handleNode = (rawFile, rawArguments) => rawFile.toLowerCase().replace(/\.exe$/, '') === 'node'
const handleNode = (rawFile, rawArguments) => ['node', 'node.exe'].includes(rawFile.toLowerCase())
? [process.execPath, [...process.execArgv.filter(flag => !flag.startsWith('--inspect')), ...rawArguments]]
: [rawFile, rawArguments];

Expand All @@ -46,8 +46,7 @@ const bufferOutput = (stream, {state}, streamName) => {

state.isIterating = false;
stream.on('data', chunk => {
for (const outputName of [streamName, 'output']) {
state[outputName] += chunk;
}
state[streamName] += chunk;
state.output += chunk;
});
};
4 changes: 2 additions & 2 deletions test/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {red} from 'yoctocolors';
import nanoSpawn from '../source/index.js';
import {testString} from './helpers/arguments.js';
import {assertDurationMs} from './helpers/assert.js';
import {nodePrint, nodePrintStdout} from './helpers/commands.js';
import {nodePrint, nodePrintFail, nodePrintStdout} from './helpers/commands.js';

test('result.command does not quote normal arguments', async t => {
const {command} = await nanoSpawn('node', ['--version']);
Expand All @@ -27,6 +27,6 @@ test('result.durationMs is set', async t => {
});

test('error.durationMs is set', async t => {
const {durationMs} = await t.throwsAsync(nanoSpawn('node', ['--unknown']));
const {durationMs} = await t.throwsAsync(nanoSpawn(...nodePrintFail));
assertDurationMs(t, durationMs);
});
2 changes: 2 additions & 0 deletions test/fixtures/node_modules/.bin/git

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions test/fixtures/subdir/node_modules/.bin/git

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/helpers/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {nonExistentCommand, nodeHangingCommand, nodeEvalCommandStart} from './co

export const assertDurationMs = (t, durationMs) => {
t.true(Number.isFinite(durationMs));
t.true(durationMs > 0);
t.true(durationMs >= 0);
};

export const assertNonExistent = (t, {exitCode, signalName, command, message, stderr, cause, durationMs}, commandStart = nonExistentCommand, expectedCommand = commandStart) => {
Expand Down
23 changes: 17 additions & 6 deletions test/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,20 @@
import test from 'ava';
import nanoSpawn from '../source/index.js';
import {assertSigterm} from './helpers/assert.js';
import {nodePrintStdout, nodeHanging} from './helpers/commands.js';
import {assertSigterm} from './helpers/assert.js';

test('Can pass no arguments', async t => {
const error = await t.throwsAsync(nanoSpawn(...nodeHanging, {timeout: 1}));
assertSigterm(t, error);
});

test('Can pass no arguments nor options', async t => {
const promise = nanoSpawn(...nodeHanging);
const nodeChildProcess = await promise.nodeChildProcess;
nodeChildProcess.kill();
const error = await t.throwsAsync(promise);
assertSigterm(t, error);
});

test('Returns a promise', async t => {
const promise = nanoSpawn(...nodePrintStdout);
Expand All @@ -12,10 +25,8 @@ test('Returns a promise', async t => {
});

test('promise.nodeChildProcess is set', async t => {
const promise = nanoSpawn(...nodeHanging);
const promise = nanoSpawn(...nodePrintStdout);
const nodeChildProcess = await promise.nodeChildProcess;
nodeChildProcess.kill();

const error = await t.throwsAsync(promise);
assertSigterm(t, error);
t.true(Number.isInteger(nodeChildProcess.pid));
await promise;
});
66 changes: 48 additions & 18 deletions test/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
fixturesPath,
nodeDirectory,
} from './helpers/main.js';
import {testString} from './helpers/arguments.js';
import {testString, secondTestString} from './helpers/arguments.js';
import {
assertNonExistent,
assertWindowsNonExistent,
Expand Down Expand Up @@ -45,6 +45,14 @@ const testArgv0 = async (t, shell) => {
test('Can pass options.argv0', testArgv0, false);
test('Can pass options.argv0, shell', testArgv0, true);

const testCwd = async (t, cwd) => {
const {stdout} = await nanoSpawn(...nodePrint('process.cwd()'), {cwd});
t.is(stdout, fixturesPath.replace(/[\\/]$/, ''));
};

test('Can pass options.cwd string', testCwd, fixturesPath);
test('Can pass options.cwd URL', testCwd, FIXTURES_URL);

const testStdOption = async (t, optionName) => {
const promise = nanoSpawn(...nodePrintStdout, {[optionName]: 'ignore'});
const subprocess = await promise.nodeChildProcess;
Expand All @@ -56,13 +64,24 @@ test('Can pass options.stdin', testStdOption, 'stdin');
test('Can pass options.stdout', testStdOption, 'stdout');
test('Can pass options.stderr', testStdOption, 'stderr');

const testStdOptionDefault = async (t, optionName) => {
const promise = nanoSpawn(...nodePrintStdout);
const subprocess = await promise.nodeChildProcess;
t.not(subprocess[optionName], null);
await promise;
};

test('options.stdin defaults to "pipe"', testStdOptionDefault, 'stdin');
test('options.stdout defaults to "pipe"', testStdOptionDefault, 'stdout');
test('options.stderr defaults to "pipe"', testStdOptionDefault, 'stderr');

test('Can pass options.stdio array', async t => {
const promise = nanoSpawn(...nodePrintStdout, {stdio: ['ignore', 'pipe', 'pipe', 'pipe']});
const {stdin, stdout, stderr, stdio} = await promise.nodeChildProcess;
t.is(stdin, null);
t.not(stdout, null);
t.not(stderr, null);
t.not(stdio[3], null);
t.is(stdio.length, 4);
await promise;
});

Expand All @@ -76,29 +95,23 @@ test('Can pass options.stdio string', async t => {
await promise;
});

test('options.stdio array has priority over options.stdout', async t => {
const promise = nanoSpawn(...nodePrintStdout, {stdio: ['pipe', 'pipe', 'pipe'], stdout: 'ignore'});
const testStdioPriority = async (t, stdio) => {
const promise = nanoSpawn(...nodePrintStdout, {stdio, stdout: 'ignore'});
const {stdout} = await promise.nodeChildProcess;
t.not(stdout, null);
await promise;
});
};

test('options.stdio string has priority over options.stdout', async t => {
const promise = nanoSpawn(...nodePrintStdout, {stdio: 'pipe', stdout: 'ignore'});
const {stdout} = await promise.nodeChildProcess;
t.not(stdout, null);
await promise;
});
test('options.stdio array has priority over options.stdout', testStdioPriority, ['pipe', 'pipe', 'pipe']);
test('options.stdio string has priority over options.stdout', testStdioPriority, 'pipe');

test('options.stdin can be {string: string}', async t => {
const {stdout} = await nanoSpawn(...nodePassThrough, {stdin: {string: testString}});
const testInput = async (t, options) => {
const {stdout} = await nanoSpawn(...nodePassThrough, options);
t.is(stdout, testString);
});
};

test('options.stdio[0] can be {string: string}', async t => {
const {stdout} = await nanoSpawn(...nodePassThrough, {stdio: [{string: testString}, 'pipe', 'pipe']});
t.is(stdout, testString);
});
test('options.stdin can be {string: string}', testInput, {stdin: {string: testString}});
test('options.stdio[0] can be {string: string}', testInput, {stdio: [{string: testString}, 'pipe', 'pipe']});

const testLocalBinaryExec = async (t, cwd) => {
const {stdout} = await nanoSpawn(...localBinary, {preferLocal: true, cwd});
Expand Down Expand Up @@ -142,6 +155,11 @@ test('options.preferLocal true uses options.env when empty', async t => {
}
});

test('options.preferLocal true can use an empty PATH', async t => {
const {stdout} = await nanoSpawn(process.execPath, ['--version'], {preferLocal: true, env: {PATH: undefined, Path: undefined}});
t.is(stdout, process.version);
});

test('options.preferLocal true does not add node_modules/.bin if already present', async t => {
const localDirectory = fileURLToPath(new URL('node_modules/.bin', import.meta.url));
const currentPath = process.env[pathKey()];
Expand Down Expand Up @@ -186,6 +204,18 @@ test('options.preferLocal true can pass arguments to local npm binaries, space',
test('options.preferLocal true can pass arguments to local npm binaries, *', testLocalBinary, '*');
test('options.preferLocal true can pass arguments to local npm binaries, ?', testLocalBinary, '?');

if (!isWindows) {
test('options.preferLocal true prefer local binaries over global ones', async t => {
const {stdout} = await nanoSpawn('git', {preferLocal: true, cwd: FIXTURES_URL});
t.is(stdout, testString);
});

test('options.preferLocal true prefer subdirectories over parent directories', async t => {
const {stdout} = await nanoSpawn('git', {preferLocal: true, cwd: new URL('subdir', FIXTURES_URL)});
t.is(stdout, secondTestString);
});
}

test('Can run global npm binaries', async t => {
const {stdout} = await nanoSpawn('npm', ['--version']);
t.regex(stdout, VERSION_REGEXP);
Expand Down

0 comments on commit 0b1672d

Please sign in to comment.