@samitouri / QOS-React-2 / commits / a8db941fb7

[snap tester] Handle interrupts by forking runner

Run snap tester on a forked node process so runs can be interrupted. (Currently, `Ctrl+C` is not handled until after all test fixtures finish compiling). This *feels* a bit heavy-handed, but main.ts is pretty small and doesn't do much other than listen for input / signals. Would love feedback here since I haven't really worked with nodejs / JS cli tools before - main.ts new file that spawns forked runner - pipe stdin/out/err to and from the child runner process, details described in comments. - listens for exit event of child - runner.ts - added logic to listen for interrupts and clean up (not really familiar with how file and tsc watchers are implemented, so we try to call 'close' on them just in case they need to release locks / do other cleanup)

Mofei Zhang committed Apr 24, 2023 at 19:09 UTC a8db941fb7de28f72e0725dee89b6a298683b3a7
4 files changed +131 -32
compiler/forget/package.json
+1 -1
@@ -14,7 +14,7 @@
14 "hash": "scripts/hash-dist.sh",
15 "playground": "cd packages/playground && yarn && yarn dev",
16 "test": "tsc && jest",
17 - "snap": "node packages/snap/dist/runner.js",
17 + "snap": "node packages/snap/dist/main.js",
18 "snap:build": "cd packages/snap && yarn build",
19 "ts:analyze-trace": "scripts/ts-analyze-trace.sh",
20 "test262": "yarn run --silent test262-harness --preprocessor=scripts/test262-preprocessor.js",
compiler/forget/packages/snap/package.json
+1 -1
@@ -3,7 +3,7 @@
3 "version": "0.0.1",
4 "public": false,
5 "description": "Snapshot testing CLI tool",
6 - "main": "dist/index.js",
6 + "main": "dist/main.js",
7 "license": "MIT",
8 "files": [
9 "src"
compiler/forget/packages/snap/src/main.ts new
+50
@@ -0,0 +1,50 @@
1 +/**
2 + * Copyright (c) Meta Platforms, Inc. and affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + */
7 +
8 +import { fork } from "child_process";
9 +import invariant from "invariant";
10 +import process from "process";
11 +import * as readline from "readline";
12 +import { hideBin } from "yargs/helpers";
13 +
14 +readline.emitKeypressEvents(process.stdin);
15 +
16 +if (process.stdin.isTTY) {
17 + process.stdin.setRawMode(true);
18 +}
19 +
20 +process.stdin.on("keypress", function (chunk, key) {
21 + if (key && key.name === "c" && key.ctrl) {
22 + // handle sigint
23 + if (childProc) {
24 + console.log("Interrupted!!");
25 + childProc.kill("SIGINT");
26 + childProc.unref();
27 + process.exit(-1);
28 + }
29 + }
30 +});
31 +
32 +const childProc = fork(require.resolve("./runner.js"), hideBin(process.argv), {
33 + // for some reason, keypress events aren't sent to handlers in both processes
34 + // when we `inherit` stdin.
35 + // pipe stdout and stderr so we can silence child process after parent exits
36 + stdio: ["pipe", "pipe", "pipe", "ipc"],
37 + env: { FORCE_COLOR: "true" },
38 +});
39 +
40 +invariant(
41 + childProc.stdin && childProc.stdout && childProc.stderr,
42 + "Expected forked process to have piped stdio"
43 +);
44 +process.stdin.pipe(childProc.stdin);
45 +childProc.stdout.pipe(process.stdout);
46 +childProc.stderr.pipe(process.stderr);
47 +
48 +childProc.on("exit", (code) => {
49 + process.exit(code ?? -1);
50 +});
compiler/forget/packages/snap/src/runner.ts
+79 -30
@@ -21,16 +21,29 @@ import * as compiler from "./compiler-worker.js";
21
22 readline.emitKeypressEvents(process.stdin);
23
24 -if (process.stdin.isTTY) {
25 - process.stdin.setRawMode(true);
26 -}
24 +process.stdin.on("keypress", function (chunk, key) {
25 + if (key && key.name === "c" && key.ctrl) {
26 + cleanup(-1);
27 + }
28 +});
29 +process.on("SIGINT", function () {
30 + // Parent process may send SIGINT
31 + cleanup(-1);
32 +});
33 +
34 +process.on("SIGTERM", function () {
35 + cleanup(-1);
36 +});
37
28 -const argv: {
38 +type Results = Map<string, TestResult>;
39 +type RunnerOptions = {
40 sync: boolean;
41 workerThreads: boolean;
42 watch: boolean;
43 update: boolean;
33 -} = yargs
44 +};
45 +
46 +const opts: RunnerOptions = yargs
47 .boolean("sync")
48 .describe(
49 "sync",
@@ -56,10 +69,6 @@ const argv: {
69 .strict()
70 .parseSync(hideBin(process.argv));
71
59 -const PARALLEL = !argv.sync;
60 -const ENABLE_WORKER_THREADS = argv.workerThreads;
61 -const WATCH = argv.watch;
62 -const UPDATE = argv.update;
72 const WORKER_PATH = require.resolve("./compiler-worker.js");
73 const COMPILER_PATH = path.join(
74 process.cwd(),
@@ -75,18 +84,33 @@ const FIXTURES_PATH = path.join(
84 "compiler"
85 );
86
78 -const worker: Worker & typeof compiler = new Worker(WORKER_PATH, {
79 - enableWorkerThreads: ENABLE_WORKER_THREADS,
80 -}) as any;
81 -worker.getStderr().pipe(process.stderr);
82 -worker.getStdout().pipe(process.stdout);
83 -
84 -type Results = Map<string, TestResult>;
87 +/**
88 + * Cleanup / handle interrupts
89 + */
90 +const cleanupTasks: Array<() => void> = new Array();
91 +function pushCleanupTask(fn: () => void) {
92 + cleanupTasks.push(fn);
93 +}
94 +function cleanup(code: number) {
95 + for (const task of cleanupTasks) {
96 + task();
97 + }
98 + process.exit(code);
99 +}
100 +function clearConsole() {
101 + // console.clear() only works when stdout is connected to a TTY device.
102 + // we're currently piping stdout (see main.ts), so let's do a 'hack'
103 + console.log("\u001Bc");
104 +}
105
106 /**
107 * Do a test run and return the test results
108 */
89 -async function run(compilerVersion: number): Promise<Results> {
109 +async function run(
110 + worker: Worker & typeof compiler,
111 + opts: RunnerOptions,
112 + compilerVersion: number
113 +): Promise<Results> {
114 // We could in theory be fancy about tracking the contents of the fixtures
115 // directory via our file subscription, but it's simpler to just re-read
116 // the directory each time.
@@ -100,7 +124,7 @@ async function run(compilerVersion: number): Promise<Results> {
124 ).sort();
125
126 let entries: Array<[string, TestResult]>;
103 - if (PARALLEL) {
127 + if (!opts.sync) {
128 // Note: promise.all to ensure parallelism when enabled
129 entries = await Promise.all(
130 fixtures.map(async (fixture) => {
@@ -288,8 +312,17 @@ enum Mode {
312 /**
313 * Runs the compiler in watch or single-execution mode
314 */
291 -async function main(): Promise<void> {
292 - if (WATCH) {
315 +export async function main(opts: RunnerOptions): Promise<void> {
316 + const worker: Worker & typeof compiler = new Worker(WORKER_PATH, {
317 + enableWorkerThreads: opts.workerThreads,
318 + }) as any;
319 + worker.getStderr().pipe(process.stderr);
320 + worker.getStdout().pipe(process.stdout);
321 + pushCleanupTask(() => {
322 + worker.end();
323 + });
324 +
325 + if (opts.watch) {
326 // Monotonically increasing integer to describe the 'version' of the compiler.
327 // This is passed to `compile()` (from compiler-worker) when compiling, so
328 // that the worker knows when it has to reset its module cache and when its
@@ -306,10 +339,10 @@ async function main(): Promise<void> {
339 async function onChange({ mode }: { mode: Mode }) {
340 if (isCompilerValid) {
341 const start = performance.now();
309 - console.clear();
342 + clearConsole();
343 console.log("Running tests...");
311 - const results = await run(compilerVersion);
312 - console.clear();
344 + const results = await run(worker, opts, compilerVersion);
345 + clearConsole();
346 if (mode === Mode.Update) {
347 update(results);
348 } else {
@@ -331,7 +364,7 @@ async function main(): Promise<void> {
364 }
365
366 // Run TS in incremental watch mode
334 - const _tsWatch = watchSrc(onStart, (isSuccess) => {
367 + const tsWatch = watchSrc(onStart, (isSuccess) => {
368 // Bump the compiler version after a build finishes
369 // and re-run tests
370 if (isSuccess) {
@@ -340,12 +373,15 @@ async function main(): Promise<void> {
373 isCompilerValid = isSuccess;
374 onChange({ mode: Mode.Test });
375 });
376 + pushCleanupTask(() => {
377 + tsWatch.close();
378 + });
379
380 // Watch the fixtures directory for changes
381 // TODO: ignore changes that occurred as a result of our explicitly updating
382 // fixtures in update() - maybe keep a timestamp of last known changes, and
383 // ignore events that occurred prior to that timestamp.
348 - const _fileSubscription = watcher.subscribe(
384 + const fileSubscription = watcher.subscribe(
385 FIXTURES_PATH,
386 async (err, _events) => {
387 if (err) {
@@ -357,6 +393,16 @@ async function main(): Promise<void> {
393 }
394 );
395
396 + pushCleanupTask(() => {
397 + fileSubscription
398 + .then((subscription) => {
399 + subscription.unsubscribe();
400 + })
401 + .catch((err) => {
402 + console.log("error cleaning up file subscription", err);
403 + });
404 + });
405 +
406 // Basic key event handling
407 process.stdin.on("keypress", (str, key) => {
408 if (key.name === "u") {
@@ -364,8 +410,6 @@ async function main(): Promise<void> {
410 onChange({ mode: Mode.Update });
411 } else if (key.name === "q") {
412 process.exit(0);
367 - } else if (key.ctrl && key.name === "c") {
368 - process.exit(0);
413 } else {
414 // any other key re-runs tests
415 onChange({ mode: Mode.Test });
@@ -380,8 +424,8 @@ async function main(): Promise<void> {
424 () => {},
425 async (isSuccess: boolean) => {
426 if (isSuccess) {
383 - const results = await run(0);
384 - if (UPDATE) {
427 + const results = await run(worker, opts, 0);
428 + if (opts.update) {
429 update(results);
430 } else {
431 report(results);
@@ -393,14 +437,19 @@ async function main(): Promise<void> {
437 }
438 if (tsWatch != null) {
439 tsWatch.close();
440 + tsWatch = null;
441 }
442 await worker.end();
443 process.exit(isSuccess ? 0 : -1);
444 }
445 );
446 + pushCleanupTask(() => {
447 + tsWatch?.close();
448 + tsWatch = null;
449 + });
450 }
451 }
452
453 // I couldn't figure out the right combination of settings to allow using `await` at the top-level,
454 // but it's easy enough to use the promise API just here
406 -main().catch((error) => console.error(error));
455 +main(opts).catch((error) => console.error(error));