mirror of
https://github.com/LukeHagar/vercel.git
synced 2025-12-10 04:22:12 +00:00
[cli][next] Removed --forceExit from jest args (#8718)
Remove `--forceExit` and wait for the child `vc dev` process to close before continuing. Listen to the `'close'` event instead of `'exit'` to ensure stdio has been flushed and closed before continuing. Some tests cause `vc dev` to spawn child processes that in turn spawn nested child processes that do not exit when the parent exits, so a `nukeProcessTree()` helper was added to ensure all spawned processes are cleaned up. Furthermore, some of the nested child processes don't respond well to `SIGTERM` and we need to use `SIGKILL` if `SIGTERM` doesn't do the job. I uncovered a larger issue exposed by removing the `--forceExit` flag. The go builder uses `go run` to build and run the go-based serverless function. Turns out that `go run` will build the executable, then spawn it with a parent process id (ppid) of `1` (launchd) on macOS. This means that the go serverless function executable is not detected as a child process of `vc dev` and won't be cleaned up, yet has inherited `stdout` and `stderr` keeping Jest alive. The solution is to explicitly `go build` the serverless executable, then run it. ### 📋 Checklist <!-- Please keep your PR as a Draft until the checklist is complete --> #### Tests - [x] The code changed/added as part of this PR has been covered with tests - [x] All tests pass locally with `yarn test-unit` #### Code Review - [ ] This PR has a concise title and thorough description useful to a reviewer - [ ] Issue from task tracker has a link to this PR Co-authored-by: Sean Massa <EndangeredMassa@gmail.com>
This commit is contained in:
Chris Barber
@@ -10,6 +10,7 @@ const { version: cliVersion } = require('../../package.json');
|
||||
const {
|
||||
fetchCachedToken,
|
||||
} = require('../../../../test/lib/deployment/now-deploy');
|
||||
const { spawnSync } = require('child_process');
|
||||
|
||||
jest.setTimeout(6 * 60 * 1000);
|
||||
|
||||
@@ -33,7 +34,7 @@ function execa(...args) {
|
||||
const child = _execa(...args);
|
||||
|
||||
processList.set(procId, child);
|
||||
child.on('exit', () => processList.delete(procId));
|
||||
child.on('close', () => processList.delete(procId));
|
||||
|
||||
return child;
|
||||
}
|
||||
@@ -212,7 +213,6 @@ async function testFixture(directory, opts = {}, args = []) {
|
||||
],
|
||||
{
|
||||
reject: false,
|
||||
detached: true,
|
||||
shell: true,
|
||||
stdio: 'pipe',
|
||||
...opts,
|
||||
@@ -240,8 +240,22 @@ async function testFixture(directory, opts = {}, args = []) {
|
||||
});
|
||||
|
||||
let printedOutput = false;
|
||||
let devTimer = null;
|
||||
|
||||
dev.on('exit', () => {
|
||||
dev.on('exit', code => {
|
||||
devTimer = setTimeout(async () => {
|
||||
const pids = Object.keys(await ps(dev.pid)).join(', ');
|
||||
console.error(
|
||||
`Test ${directory} exited with code ${code}, but has timed out closing stdio\n` +
|
||||
(pids
|
||||
? `Hanging child processes: ${pids}`
|
||||
: `${dev.pid} already exited`)
|
||||
);
|
||||
}, 5000);
|
||||
});
|
||||
|
||||
dev.on('close', () => {
|
||||
clearTimeout(devTimer);
|
||||
if (!printedOutput) {
|
||||
printOutput(directory, stdout, stderr);
|
||||
printedOutput = true;
|
||||
@@ -260,8 +274,12 @@ async function testFixture(directory, opts = {}, args = []) {
|
||||
});
|
||||
|
||||
dev._kill = dev.kill;
|
||||
dev.kill = async (...args) => {
|
||||
dev._kill(...args);
|
||||
dev.kill = async () => {
|
||||
// kill the entire process tree for the child as some tests will spawn
|
||||
// child processes that either become defunct or assigned a new parent
|
||||
// process
|
||||
await nukeProcessTree(dev.pid);
|
||||
|
||||
await exitResolver;
|
||||
return {
|
||||
stdout,
|
||||
@@ -443,7 +461,7 @@ function testFixtureStdio(
|
||||
stdout += data;
|
||||
});
|
||||
|
||||
dev.stderr.on('data', data => {
|
||||
dev.stderr.on('data', async data => {
|
||||
stderr += data;
|
||||
|
||||
if (stripAnsi(data).includes('Ready! Available at')) {
|
||||
@@ -452,19 +470,19 @@ function testFixtureStdio(
|
||||
}
|
||||
|
||||
if (stderr.includes(`Requested port ${port} is already in use`)) {
|
||||
dev.kill('SIGTERM');
|
||||
await nukeProcessTree(dev.pid);
|
||||
throw new Error(
|
||||
`Failed for "${directory}" with port ${port} with stderr "${stderr}".`
|
||||
);
|
||||
}
|
||||
|
||||
if (stderr.includes('Command failed')) {
|
||||
dev.kill('SIGTERM');
|
||||
await nukeProcessTree(dev.pid);
|
||||
throw new Error(`Failed for "${directory}" with stderr "${stderr}".`);
|
||||
}
|
||||
});
|
||||
|
||||
dev.on('exit', () => {
|
||||
dev.on('close', () => {
|
||||
if (!printedOutput) {
|
||||
printOutput(directory, stdout, stderr);
|
||||
printedOutput = true;
|
||||
@@ -490,12 +508,80 @@ function testFixtureStdio(
|
||||
};
|
||||
await fn(helperTestPath, port);
|
||||
} finally {
|
||||
dev.kill('SIGTERM');
|
||||
await nukeProcessTree(dev.pid);
|
||||
await exitResolver;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
async function ps(parentPid, pids = {}) {
|
||||
const cmd =
|
||||
process.platform === 'darwin'
|
||||
? ['pgrep', '-P', parentPid]
|
||||
: ['ps', '-o', 'pid', '--no-headers', '--ppid', parentPid];
|
||||
|
||||
try {
|
||||
const { stdout: buf } = spawnSync(cmd[0], cmd.slice(1), {
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
for (let pid of buf.match(/\d+/g)) {
|
||||
pid = parseInt(pid);
|
||||
const recurse = Object.prototype.hasOwnProperty.call(pids, pid);
|
||||
pids[parentPid].push(pid);
|
||||
pids[pid] = [];
|
||||
if (recurse) {
|
||||
await ps(pid, pids);
|
||||
}
|
||||
}
|
||||
} catch (e) {
|
||||
console.log(`Failed to get processes: ${e.toString()}`);
|
||||
}
|
||||
return pids;
|
||||
}
|
||||
|
||||
async function nukePID(pid, signal = 'SIGTERM', retries = 10) {
|
||||
if (retries === 0) {
|
||||
console.log(`pid ${pid} won't die, giving up`);
|
||||
return;
|
||||
}
|
||||
|
||||
// kill the process
|
||||
try {
|
||||
process.kill(pid, signal);
|
||||
} catch (e) {
|
||||
// process does not exist
|
||||
console.log(`pid ${pid} is not running`);
|
||||
return;
|
||||
}
|
||||
|
||||
await sleep(250);
|
||||
|
||||
try {
|
||||
// check if killed
|
||||
process.kill(pid, 0);
|
||||
} catch (e) {
|
||||
console.log(`pid ${pid} is not running`);
|
||||
return;
|
||||
}
|
||||
|
||||
console.log(`pid ${pid} didn't exit, sending SIGKILL (retries ${retries})`);
|
||||
await nukePID(pid, 'SIGKILL', retries - 1);
|
||||
}
|
||||
|
||||
async function nukeProcessTree(pid, signal) {
|
||||
if (process.platform === 'win32') {
|
||||
spawnSync('taskkill', ['/pid', pid, '/T', '/F'], { stdio: 'inherit' });
|
||||
return;
|
||||
}
|
||||
|
||||
const pids = await ps(pid, {
|
||||
[pid]: [],
|
||||
});
|
||||
|
||||
console.log(`Nuking pids: ${Object.keys(pids).join(', ')}`);
|
||||
await Promise.all(Object.keys(pids).map(pid => nukePID(pid, signal)));
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
port = ++port;
|
||||
});
|
||||
@@ -503,17 +589,15 @@ beforeEach(() => {
|
||||
afterEach(async () => {
|
||||
await Promise.all(
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
Array.from(processList).map(([_procId, proc]) => {
|
||||
if (proc.killed === false) {
|
||||
console.log(
|
||||
`killing process ${proc.pid} "${proc.spawnargs.join(' ')}"`
|
||||
);
|
||||
Array.from(processList).map(async ([_procId, proc]) => {
|
||||
console.log(`killing process ${proc.pid} "${proc.spawnargs.join(' ')}"`);
|
||||
|
||||
try {
|
||||
process.kill(proc.pid, 'SIGTERM');
|
||||
} catch (err) {
|
||||
// Was already killed
|
||||
console.error(`Failed to kill process`, proc.pid, err);
|
||||
try {
|
||||
await nukeProcessTree(proc.pid);
|
||||
} catch (err) {
|
||||
// Was already killed
|
||||
if (err.code !== 'ESRCH') {
|
||||
console.error('Failed to kill process', proc.pid, err);
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user