Skip to content

Commit 42f609c

Browse files
divybotlittledivy
andauthored
fix(ext/node): node:test with watch-mode events (#34254)
## Summary - Replaces `notImplemented("test.run")` in `ext/node/polyfills/testing.ts` with a real `TestsStream`-compatible `Readable`. - Supports `run({ watch, cwd, signal })`: emits `test:watch:drained` after each run cycle, and `test:watch:restarted` (followed by `drained`) when a file changes under the watched cwd. - The returned stream both pushes events as data chunks (for async iteration / `'data'` listeners) and emits each event under its `type` as a named event, matching Node's `TestsStream` semantics so callers can do `stream.on('test:watch:drained', ...)` directly. - `AbortSignal` ends the stream and closes the watcher; bursts of fs events are debounced (~50ms) so a single user-visible change triggers exactly one restart cycle. ## Scope This PR implements the watch-mode event machinery only. Programmatic test discovery / execution under `run()` (which would let it actually emit `test:pass`/`test:fail`) is deliberately left for a follow-up; the stream emits empty run cycles for now. That is enough to unblock the watch-event fixtures in the Node compat suite that drive behavior through the programmatic API rather than spawning a subprocess. Five previously-absent test-runner watch fixtures now pass and are added to \`tests/node_compat/config.jsonc\`: - \`test-runner/test-run-watch-cwd.mjs\` - \`test-runner/test-run-watch-cwd-isolation-none.mjs\` - \`test-runner/test-run-watch-cwd-isolation-process.mjs\` - \`test-runner/test-run-watch-emit-restarted.mjs\` (validates the drained/restarted/drained sequence via \`assert.partialDeepStrictEqual\`) - \`test-runner/test-run-watch-no-emit-restarted-disabled.mjs\` (validates that \`watch: false\` never emits \`test:watch:restarted\` via \`common.mustNotCall\`) The remaining \`test-run-watch-*\` and \`test-watch-*-isolation-*\` fixtures depend on subprocess spawning of a \`run()\`-fixture piped through \`node:test/reporters\` \`tap\`, or on CLI \`--test --watch\` flags - both substantial separate features and out of scope for this PR. Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent 0027351 commit 42f609c

7 files changed

Lines changed: 229 additions & 2 deletions

File tree

ext/node/polyfills/testing.ts

Lines changed: 136 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,142 @@ function getAssertObject() {
120120
return assertObject;
121121
}
122122

123-
function run() {
124-
notImplemented("test.run");
123+
// Lazy access to other node polyfills; loading these eagerly at module
124+
// init causes circular initialization issues during snapshotting.
125+
let _Readable = null;
126+
function getReadable() {
127+
if (_Readable === null) {
128+
_Readable = core.loadExtScript(
129+
"ext:deno_node/internal/streams/readable.js",
130+
).Readable;
131+
}
132+
return _Readable;
133+
}
134+
let _fsWatch = null;
135+
function getFsWatch() {
136+
if (_fsWatch === null) {
137+
_fsWatch = core.loadExtScript("ext:deno_node/fs.ts").watch;
138+
}
139+
return _fsWatch;
140+
}
141+
const lazyProcess = core.createLazyLoader("node:process");
142+
143+
// node:test `run()` implementation.
144+
//
145+
// Returns a `TestsStream`-compatible Readable that emits structured events
146+
// describing the test run lifecycle. We currently support the watch-mode
147+
// event stream (`test:watch:drained`, `test:watch:restarted`) which is the
148+
// minimum required for the Node.js `test-runner/test-run-watch-*` fixtures
149+
// that drive watch behavior through the programmatic API. Actual test file
150+
// discovery / execution remains TODO and is gated behind separate work; the
151+
// stream emits a single empty run cycle, then either ends (watch:false) or
152+
// waits for filesystem changes to trigger restarts (watch:true).
153+
//
154+
// See test-runner/test-run-watch-*.mjs in the Node compat suite for the
155+
// behavior this implements.
156+
function run(options) {
157+
options = options ?? {};
158+
const watch = options.watch === true;
159+
const signal = options.signal;
160+
let cwd = options.cwd;
161+
if (cwd === undefined) {
162+
cwd = lazyProcess().default.cwd();
163+
}
164+
165+
const Readable = getReadable();
166+
const stream = new Readable({
167+
__proto__: null,
168+
objectMode: true,
169+
// We push events imperatively; the consumer just needs a no-op `_read`.
170+
read() {},
171+
});
172+
173+
let watcher = null;
174+
let finished = false;
175+
let pendingRestartTimer = null;
176+
177+
function finish() {
178+
if (finished) return;
179+
finished = true;
180+
if (pendingRestartTimer !== null) {
181+
clearTimeout(pendingRestartTimer);
182+
pendingRestartTimer = null;
183+
}
184+
if (watcher !== null) {
185+
try {
186+
watcher.close();
187+
} catch { /* ignore */ }
188+
watcher = null;
189+
}
190+
stream.push(null);
191+
}
192+
193+
function emit(type) {
194+
if (finished) return;
195+
const data = { __proto__: null };
196+
// Node's TestsStream emits each lifecycle entry both as a data chunk
197+
// (consumed via async iteration / `'data'` listeners) and as a named
198+
// event so callers can attach `.on('test:watch:drained', ...)` directly.
199+
stream.push({ __proto__: null, type, data });
200+
stream.emit(type, data);
201+
}
202+
203+
function drained() {
204+
emit("test:watch:drained");
205+
}
206+
207+
function scheduleRestart() {
208+
if (finished) return;
209+
// Debounce bursts of fs events so a single user-visible change produces
210+
// exactly one restart cycle (Node's watcher coalesces likewise).
211+
if (pendingRestartTimer !== null) {
212+
clearTimeout(pendingRestartTimer);
213+
}
214+
pendingRestartTimer = setTimeout(() => {
215+
pendingRestartTimer = null;
216+
if (finished) return;
217+
emit("test:watch:restarted");
218+
drained();
219+
}, 50);
220+
}
221+
222+
if (signal) {
223+
if (signal.aborted) {
224+
// Resolve the initial drained on next tick to keep callers that
225+
// `await once(stream, 'test:watch:drained')` working.
226+
queueMicrotask(() => {
227+
drained();
228+
finish();
229+
});
230+
return stream;
231+
}
232+
signal.addEventListener("abort", finish, { once: true });
233+
}
234+
235+
// Emit the initial "drained" event after the current microtask completes
236+
// so that consumers attaching `.on('data')` synchronously after `run(...)`
237+
// returns still observe the event.
238+
queueMicrotask(() => {
239+
drained();
240+
if (!watch) {
241+
finish();
242+
return;
243+
}
244+
try {
245+
const fsWatch = getFsWatch();
246+
watcher = fsWatch(cwd, { recursive: true }, () => {
247+
scheduleRestart();
248+
});
249+
watcher.on("error", () => {
250+
finish();
251+
});
252+
} catch {
253+
// If we can't watch (e.g. cwd doesn't exist), end the stream gracefully.
254+
finish();
255+
}
256+
});
257+
258+
return stream;
125259
}
126260

127261
function noop() {}

tests/node_compat/config.jsonc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4457,6 +4457,11 @@
44574457
"reason": "Runs without the previous missing test.expectFailure TypeError, but reporter snapshot output still differs from Node"
44584458
},
44594459
"test-runner/test-output-typescript-coverage.mjs": {},
4460+
"test-runner/test-run-watch-cwd-isolation-none.mjs": {},
4461+
"test-runner/test-run-watch-cwd-isolation-process.mjs": {},
4462+
"test-runner/test-run-watch-cwd.mjs": {},
4463+
"test-runner/test-run-watch-emit-restarted.mjs": {},
4464+
"test-runner/test-run-watch-no-emit-restarted-disabled.mjs": {},
44604465
// TODO(bartlomieju): disabled during work on `node:inspector`, this test didn't actualy run before
44614466
// "sequential/test-watch-mode-inspect.mjs": {},
44624467
// TODO(bartlomieju): this test is very flaky on CI, especially on Windows ARM
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"tests": {
3+
"watch_disabled": {
4+
"args": "run -A watch_disabled.mjs",
5+
"output": "watch_disabled.out"
6+
},
7+
"watch_restart": {
8+
"args": "run -A watch_restart.mjs",
9+
"output": "watch_restart.out"
10+
}
11+
}
12+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// `run({ watch: false })` should not emit `test:watch:restarted` even when
2+
// files in the watched cwd change after the run starts.
3+
import { run } from "node:test";
4+
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
5+
import { tmpdir } from "node:os";
6+
import { join } from "node:path";
7+
8+
const dir = mkdtempSync(join(tmpdir(), "node-test-run-watch-"));
9+
10+
let restarted = false;
11+
const stream = run({ cwd: dir, watch: false });
12+
stream.on("test:watch:restarted", () => {
13+
restarted = true;
14+
});
15+
16+
writeFileSync(join(dir, "test.js"), "module.exports = {};");
17+
18+
// eslint-disable-next-line no-unused-vars
19+
for await (const _ of stream);
20+
21+
rmSync(dir, { recursive: true, force: true });
22+
23+
if (restarted) {
24+
console.log("fail: test:watch:restarted was emitted");
25+
process.exit(1);
26+
}
27+
console.log("ok");
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ok
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// `run({ watch: true })` should emit `test:watch:drained` after the initial
2+
// (empty) run cycle, then emit `test:watch:restarted` followed by another
3+
// `test:watch:drained` when a file inside the watched cwd changes.
4+
import { run } from "node:test";
5+
import { once } from "node:events";
6+
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
7+
import { tmpdir } from "node:os";
8+
import { join } from "node:path";
9+
10+
const dir = mkdtempSync(join(tmpdir(), "node-test-run-watch-"));
11+
12+
const events = [];
13+
let drainCount = 0;
14+
const controller = new AbortController();
15+
16+
const stream = run({ cwd: dir, watch: true, signal: controller.signal })
17+
.on("data", ({ type }) => {
18+
events.push(type);
19+
if (type === "test:watch:drained") {
20+
drainCount++;
21+
if (drainCount >= 2) {
22+
controller.abort();
23+
}
24+
}
25+
});
26+
27+
await once(stream, "test:watch:drained");
28+
writeFileSync(join(dir, "test.js"), "module.exports = {};");
29+
30+
// eslint-disable-next-line no-unused-vars
31+
for await (const _ of stream);
32+
33+
rmSync(dir, { recursive: true, force: true });
34+
35+
const expected = [
36+
"test:watch:drained",
37+
"test:watch:restarted",
38+
"test:watch:drained",
39+
];
40+
for (let i = 0; i < expected.length; i++) {
41+
if (events[i] !== expected[i]) {
42+
console.log(`fail: events[${i}] = ${events[i]}, expected ${expected[i]}`);
43+
console.log(`full events: ${JSON.stringify(events)}`);
44+
process.exit(1);
45+
}
46+
}
47+
console.log("ok");
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ok

0 commit comments

Comments
 (0)