Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c757321
test(node-compat): refresh the corpus to Node 26.7.0 and score it und…
colinhacks Aug 22, 2026
786f846
test(node-compat): run the gate from the suite root, drain stderr con…
colinhacks Aug 22, 2026
4f9615a
test(node-compat): address review — per-worker TEST_SERIAL_ID, Deno's…
colinhacks Aug 22, 2026
f6f51da
test(node-compat): collapse any run of slashes in a reason; keep the …
colinhacks Aug 22, 2026
f6401b9
test(node-compat): record the bulk run's parallelism (6) and the merg…
colinhacks Aug 22, 2026
48f5a2c
test(node-compat): keep the bulk run's retry setting through a merge;…
colinhacks Aug 22, 2026
d491196
test(node-compat): the 14-file merge ran with the retry pass; record …
colinhacks Aug 22, 2026
bad7b7f
test(node-compat): re-run the 14-file merge so results.json's meta is…
colinhacks Aug 22, 2026
74817a6
test(node-compat): run the corpus the way Node's runner does — full c…
colinhacks Aug 22, 2026
adb7199
test(node-compat): drop a dead classifier rule and a dead regex opera…
colinhacks Aug 22, 2026
a6ff547
test(node-compat): state the measured number of regressions the sourc…
colinhacks Aug 22, 2026
780d3fe
test(cross-runtime): scrub machine paths before truncating a failure …
colinhacks Aug 22, 2026
80a2a98
test(cross-runtime): re-judge the four nub records whose bisected che…
colinhacks Aug 22, 2026
a332d47
test(cross-runtime): refresh the README and wiki figures after the re…
colinhacks Aug 22, 2026
2d5bdd4
test(cross-runtime): generate the README table from results.json; not…
colinhacks Aug 22, 2026
4c30e4e
test(cross-runtime): gate the README results table in pre-push; gener…
colinhacks Aug 22, 2026
be58b18
test(cross-runtime): block only on the checker's drift verdict; gener…
colinhacks Aug 22, 2026
cf2215d
test(cross-runtime): an unreadable results.json is a could-not-check …
colinhacks Aug 22, 2026
ef30cbf
Merge remote-tracking branch 'origin/main' into node-compat-26-7
colinhacks Aug 28, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion .githooks/pre-push
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,39 @@ MSG
fi
fi

# ── Cross-runtime results-table gate ────────────────────────────────────────
# WHY THIS EXISTS: the figures in tests/cross-runtime/README.md are generated
# from results.json by readme-table.mjs, and a retyped or stale table reached
# review three rounds running before the generator existed. `--check` is ~50ms
# and runs only when the pushed range touches tests/cross-runtime/**. Like the
# site gate, an undecidable range runs the check rather than skipping it.
#
# The push refs on stdin are read ONCE here and replayed to the site gate below.
# Bypass: NUB_SKIP_README_TABLE_CHECK=1 git push · git push --no-verify
PUSH_REFS=$(cat)
if [ -z "${NUB_SKIP_README_TABLE_CHECK:-}" ] && [ -n "$SKILLS_ROOT" ] && [ -f "$SKILLS_ROOT/tests/cross-runtime/readme-table.mjs" ]; then
table_touched=0
while read -r _local_ref local_sha _remote_ref remote_sha; do
case "$local_sha" in *[!0]*) ;; *) continue ;; esac
case "$remote_sha" in
*[!0]*) base="$remote_sha" ;;
*) base=$(git merge-base "$local_sha" origin/main 2>/dev/null) ;;
esac
[ -n "$base" ] || { table_touched=1; continue; }
changed=$(git diff --name-only "$base" "$local_sha" -- tests/cross-runtime/ 2>/dev/null) || table_touched=1
[ -n "$changed" ] && table_touched=1
done <<PUSHREFS
$PUSH_REFS
PUSHREFS
if [ "$table_touched" = 1 ]; then
if ! (cd "$SKILLS_ROOT" && node tests/cross-runtime/readme-table.mjs --check) >&2; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gate treats every non-zero exit from the checker as drift, and the checker has three non-drift ways to exit non-zero. I probed each:

Condition Exit Message the user gets
Table drifted 1 correct
node not on PATH 127 "does not match results.json"
results.json unreadable 1 (uncaught throw) "does not match results.json"
Markers missing from README 2 "does not match results.json"

The PATH case is the realistic one: a GUI git client runs hooks with a non-login PATH, and an nvm-installed node is absent from it — the push is blocked with a message pointing at a file that is perfectly fine. The marker case is worse in kind, because the suggested remedy is also a dead end: --write exits 2 on missing markers too, so following the hint reproduces the failure.

This is the one behavior in the file that departs from its own stated philosophy. :18-20 says the hook "NEVER BLOCKS ON INFRASTRUCTURE. Missing deps, absent pnpm, a failed install — all warn and exit 0"; the lat gate honors it at :166-169 and :198-201, the site gate at :270-282. This block's comment invokes the site gate by name while diverging from it.

Gating on the drift code specifically keeps the guarantee and lets a broken checker announce itself as broken:

Suggested change
if ! (cd "$SKILLS_ROOT" && node tests/cross-runtime/readme-table.mjs --check) >&2; then
if [ "$table_touched" = 1 ]; then
(cd "$SKILLS_ROOT" && node tests/cross-runtime/readme-table.mjs --check) >&2
table_rc=$?
if [ "$table_rc" = 1 ]; then
echo "pre-push: tests/cross-runtime/README.md's results table does not match results.json." >&2
echo " Regenerate it: node tests/cross-runtime/readme-table.mjs --write" >&2
exit 1
elif [ "$table_rc" != 0 ]; then
echo "pre-push: results-table check could not run (exit $table_rc); skipping." >&2
fi
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two of the three rows are closed in be58b18 (127 and 2 now warn and pass, 1 still blocks — exercised on the real hook), but an unreadable results.json still exits 1 through readme-table.mjs:16's uncaught throw, so leaving this open; details in the new review.

Pullfrog  | View workflow run | via Pullfrog | Using Claude Opus | 𝕏

echo "pre-push: tests/cross-runtime/README.md's results table does not match results.json." >&2
echo " Regenerate it: node tests/cross-runtime/readme-table.mjs --write" >&2
exit 1
fi
fi
fi

[ "${NUB_SKIP_SITE_CHECK:-}" = "1" ] && exit 0

ROOT="$SKILLS_ROOT"
Expand Down Expand Up @@ -228,7 +261,9 @@ while read -r _local_ref local_sha _remote_ref remote_sha; do
fi
changed=$(git diff --name-only "$base" "$local_sha" -- site/ 2>/dev/null) || undecidable=1
[ -n "$changed" ] && site_touched=1
done
done <<PUSHREFS
$PUSH_REFS
PUSHREFS

[ "$site_touched" = 1 ] || [ "$undecidable" = 1 ] || exit 0

Expand Down
105 changes: 76 additions & 29 deletions crates/nub-cli/tests/node_compat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,35 @@ enum RunOutcome {
/// each worker its own `TMPDIR` isolates that scratch and removes the
/// false-positive failures documented in tests/node-compat-failures/parallel.md
/// ("54 were false positives from tmpdir collisions in 20-way parallel
/// execution"). `fork_id` additionally namespaces Node's own `.tmp.<id>` dir.
/// execution"). `serial_id` is exported as `TEST_SERIAL_ID`, which is what
/// `test/common/tmpdir.js` actually keys its `.tmp.<id>` directory on — without
/// it every worker shares `.tmp.0` and each `tmpdir.refresh()` wipes a sibling's.
///
/// `flags` are the test's own `// Flags:` tokens, passed to nub exactly as
/// tests/cross-runtime/run.mjs passes them, with `NODE_SKIP_FLAG_CHECK=1` so
/// `test/common` does not re-spawn the test itself. That re-spawn goes through
/// `process.execPath`, which under nub is the real `node` binary — so without
/// this every flagged test silently ran WITHOUT nub's augmentation and the gate
/// could not see a regression in it. `cwd` is the suite ROOT (the Node checkout,
/// not its `test/` dir), as in Node's own runner: flag paths such as
/// `--experimental-loader ./test/fixtures/...` are written relative to it.
fn run_with_timeout(
nub: &Path,
test_path: &Path,
flags: &[String],
cwd: &Path,
tmp: &Path,
fork_id: usize,
serial_id: usize,
) -> RunOutcome {
let mut child = match Command::new(nub)
.args(flags)
.arg(test_path)
.current_dir(cwd)
.env("NODE_TEST_KNOWN_GLOBALS", "0")
.env("NODE_SKIP_FLAG_CHECK", "1")
.env("NO_COLOR", "1")
Comment thread
pullfrog[bot] marked this conversation as resolved.
Resolved
.env("TMPDIR", tmp)
.env("NODE_TEST_FORK_ID", fork_id.to_string())
.env("TEST_SERIAL_ID", serial_id.to_string())
.stdin(Stdio::null())
.stdout(Stdio::null())
.stderr(Stdio::piped())
Expand All @@ -60,14 +75,26 @@ fn run_with_timeout(
Err(e) => return RunOutcome::Failed(format!("spawn error: {e}")),
};

// Drain stderr WHILE the child runs. Reading it only after exit deadlocks a
// chatty test: once the pipe's buffer fills, the child blocks on its next
// write and never exits, which the gate then misreports as a hang
// (test-stream2-large-read-stall.js logs every push and read).
let drain = child.stderr.take().map(|mut s| {
std::thread::spawn(move || {
let mut stderr = String::new();
let _ = s.read_to_string(&mut stderr);
stderr
})
});
let collect = |drain: Option<std::thread::JoinHandle<String>>| {
drain.and_then(|h| h.join().ok()).unwrap_or_default()
};

let start = Instant::now();
loop {
match child.try_wait() {
Ok(Some(status)) => {
let mut stderr = String::new();
if let Some(mut s) = child.stderr.take() {
let _ = s.read_to_string(&mut stderr);
}
let stderr = collect(drain);
if status.success() {
return RunOutcome::Passed;
}
Expand All @@ -85,6 +112,7 @@ fn run_with_timeout(
if start.elapsed() >= PER_TEST_TIMEOUT {
let _ = child.kill();
let _ = child.wait();
let _ = collect(drain);
return RunOutcome::TimedOut;
}
std::thread::sleep(Duration::from_millis(25));
Expand Down Expand Up @@ -112,18 +140,26 @@ fn config_path() -> PathBuf {
Path::new(&manifest).join("../../tests/node-compat-config.jsonc")
}

fn has_internal_flags(test_path: &Path) -> bool {
/// The tokens of the test's first `// Flags:` directive, if any — Node's own
/// runner passes these to the binary, and so does the cross-runtime harness
/// that generated the config, so the gate must too or it judges a different
/// program than the one the config entry describes.
fn test_flags(test_path: &Path) -> Vec<String> {
let content = fs::read_to_string(test_path).unwrap_or_default();
let header: String = content.lines().take(20).collect::<Vec<_>>().join("\n");
header.contains("--expose-internals")
|| header.contains("--allow-natives-syntax")
|| header.contains("--expose-externalize-string")
|| header.contains("--expose-gc")
content
.lines()
.find_map(|line| line.strip_prefix("// Flags: "))
.map(|rest| rest.split_whitespace().map(str::to_owned).collect())
.unwrap_or_default()
}

struct TestEntry {
path: String,
/// A classified divergence (nub fails, node passes) with its reason.
ignore: bool,
/// A divergence the generator could not classify. Skipped like `ignore`,
/// but counted and reported apart so it cannot hide among the classified.
untriaged: bool,
}

fn load_config() -> Vec<TestEntry> {
Expand All @@ -143,18 +179,19 @@ fn load_config() -> Vec<TestEntry> {
let parsed: serde_json::Value = serde_json::from_str(&stripped).unwrap_or_default();
let obj = parsed.as_object().unwrap();

let flag = |opts: &serde_json::Value, key: &str| {
opts.get(key).and_then(|v| v.as_bool()).unwrap_or(false)
};
obj.iter()
.map(|(path, opts)| TestEntry {
path: path.clone(),
ignore: opts
.get("ignore")
.and_then(|v| v.as_bool())
.unwrap_or(false),
ignore: flag(opts, "ignore"),
untriaged: flag(opts, "untriaged"),
})
.collect()
}

/// The full Node-suite compatibility corpus — ~2,554 black-box `nub` spawns.
/// The full Node-suite compatibility corpus — ~5,200 black-box `nub` spawns.
///
/// `#[ignore]` by design: this is a CI-scale gate, not a unit test, and running
/// it inline would turn every `cargo test` (and every workflow build gate) into
Expand Down Expand Up @@ -183,32 +220,36 @@ fn node_compat_suite() {

let entries = load_config();
let nub = nub_binary();
let suite_root = suite
.parent()
.expect("suite dir has a parent")
.to_path_buf();

// Resolve which entries actually run (and tally skips) before fanning out,
// so the parallel section only does spawn work.
let mut runnable: Vec<String> = Vec::new();
let mut skipped = 0usize;
let mut untriaged = 0usize;
for entry in &entries {
if entry.ignore {
skipped += 1;
continue;
}
if entry.untriaged {
untriaged += 1;
continue;
}
let test_path = suite.join(&entry.path);
if !test_path.exists() {
eprintln!("SKIP {}: not found", entry.path);
skipped += 1;
continue;
}
if has_internal_flags(&test_path) {
eprintln!("SKIP {}: internal-only flags", entry.path);
skipped += 1;
continue;
}
runnable.push(entry.path.clone());
}

// Fan the corpus across worker threads. Sequential, this is the runtime of
// ~2,554 process spawns summed; parallel it's bounded by the slowest worker.
// ~5,200 process spawns summed; parallel it's bounded by the slowest worker.
// Each worker owns an isolated TMPDIR (see run_with_timeout) so the suite's
// shared-tmpdir tests can't cross-collide. Within a worker, entries run
// sequentially and each refreshes its own scratch, so reuse is safe.
Expand All @@ -234,14 +275,17 @@ fn node_compat_suite() {
.map(|(wid, bucket)| {
let nub = &nub;
let suite = &suite;
let suite_root = &suite_root;
scope.spawn(move || {
let tmp = std::env::temp_dir()
.join(format!("nub-compat-{}-{wid}", std::process::id()));
let _ = fs::create_dir_all(&tmp);
let mut p = 0usize;
let mut suspect: Vec<String> = Vec::new();
for rel in &bucket {
match run_with_timeout(nub, &suite.join(rel), suite, &tmp, wid) {
let test_path = suite.join(rel);
let flags = test_flags(&test_path);
match run_with_timeout(nub, &test_path, &flags, suite_root, &tmp, wid) {
RunOutcome::Passed => p += 1,
RunOutcome::Failed(_) | RunOutcome::TimedOut => {
suspect.push(rel.clone())
Expand All @@ -264,7 +308,7 @@ fn node_compat_suite() {
});

// Pass 2 — sequential RE-VERIFY. Each suspect runs ALONE on the full machine
// (single TMPDIR, fork id 0); only a failure that reproduces in isolation is a
// (single TMPDIR, serial id 0); only a failure that reproduces in isolation is a
// real failure. A suspect that now passes was a parallel-load false positive
// and is credited as passed. This is the same parallel-scan-then-confirm
// protocol the parallel.md investigation used by hand — kept honest in code so
Expand All @@ -283,7 +327,9 @@ fn node_compat_suite() {
);
}
for rel in &suspects {
match run_with_timeout(&nub, &suite.join(rel), &suite, &reverify_tmp, 0) {
let test_path = suite.join(rel);
let flags = test_flags(&test_path);
match run_with_timeout(&nub, &test_path, &flags, &suite_root, &reverify_tmp, 0) {
RunOutcome::Passed => {
passed += 1;
false_positives += 1;
Expand All @@ -306,8 +352,9 @@ fn node_compat_suite() {

eprintln!(
"\n=== Node compat: {passed}/{} passed ({failed} failed, {timed_out} timed out, \
{skipped} skipped) [{workers}-way scan, {false_positives} parallel false positive(s) \
reclassified on sequential re-verify] ===",
{skipped} skipped as classified divergences, {untriaged} skipped as UNTRIAGED \
divergences — see tests/node-compat-config.jsonc) [{workers}-way scan, \
{false_positives} parallel false positive(s) reclassified on sequential re-verify] ===",
passed + failed + timed_out
);
assert_eq!(
Expand Down
Loading
Loading