From 06dbbce6376808b42c40d26d3bafa5918b76885e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 16:01:33 +0000 Subject: [PATCH 1/2] fix(test): harden gap snapshot oracle inputs --- scripts/gap_snapshot.py | 74 +++++++++++++++++++++++++++----- scripts/parity_known_failures.py | 58 +++++++++++++++++++++++-- scripts/run_gap_tests.sh | 41 ++++++++++++++++++ test-parity/README.md | 14 +++++- test-parity/gap_snapshot.json | 4 +- 5 files changed, 172 insertions(+), 19 deletions(-) diff --git a/scripts/gap_snapshot.py b/scripts/gap_snapshot.py index 5f5b9560ff..ebc3b848ce 100755 --- a/scripts/gap_snapshot.py +++ b/scripts/gap_snapshot.py @@ -49,6 +49,7 @@ ROOT = Path(__file__).resolve().parent.parent DEFAULT_REPORT = ROOT / "test-parity/reports/latest.json" DEFAULT_SNAPSHOT = ROOT / "test-parity/gap_snapshot.json" +DEFAULT_KNOWN_FAILURES = ROOT / "test-parity/known_failures.json" TEST_DIR = ROOT / "test-files" PASS = "pass" @@ -67,7 +68,11 @@ ), "fields": { "status": "parity_fail | compile_fail | crash | node_fail | skipped", - "issue": "GitHub issue tracking this failure, or null when untriaged.", + "issue": ( + "REQUIRED before commit. GitHub issue tracking this failure. " + "Snapshot update copies it from known_failures.json when available; " + "the offline audit rejects a null placeholder." + ), "added": "ISO date (YYYY-MM-DD) the test first entered the snapshot.", "category": ( "ci-env | module-inventory | bug-open | bug-stale | gap-categorical | " @@ -103,6 +108,13 @@ def load_snapshot(path: Path) -> dict[str, dict]: return json.loads(path.read_text(encoding="utf-8")).get("tests", {}) +def load_known_failures(path: Path) -> dict[str, dict]: + if not path.exists(): + return {} + data = json.loads(path.read_text(encoding="utf-8")) + return {key: value for key, value in data.items() if key != "_schema"} + + def write_snapshot(path: Path, tests: dict[str, dict]) -> None: payload = {"_schema": SCHEMA_DOC, "tests": dict(sorted(tests.items()))} path.write_text( @@ -136,9 +148,11 @@ def merge( tests: dict[str, dict], today: str, exists=test_file_exists, + known: dict[str, dict] | None = None, ) -> dict[str, dict]: - """Fold this run's statuses in, preserving triage metadata.""" + """Fold this run's statuses in, preserving/backfilling triage metadata.""" merged = dict(tests) + known = known or {} for test_id, status in report.items(): if status == PASS: # Fixed — drop the entry so it can't outlive the bug it tracked. @@ -146,14 +160,21 @@ def merge( continue previous = merged.get(test_id, {}) entry: dict[str, object] = {"status": status} - if previous: - # Carry triage across a status change: parity_fail -> crash is - # still the same open bug. - entry.update({f: previous[f] for f in TRIAGE_FIELDS if f in previous}) - else: - entry.update( - {"issue": None, "added": today, "category": "untriaged", "reason": ""} - ) + triage = known.get(test_id, {}) + # Carry triage across a status change: parity_fail -> crash is still + # the same open bug. Fill missing/null legacy fields from the + # provenance-enforced known-failure record instead of generating a + # second, contradictory explanation of the same accepted failure. + for field in TRIAGE_FIELDS: + value = previous.get(field) if isinstance(previous, dict) else None + if value in (None, "") and isinstance(triage, dict): + value = triage.get(field) + if value not in (None, ""): + entry[field] = value + entry.setdefault("issue", None) + entry.setdefault("added", today) + entry.setdefault("category", "untriaged") + entry.setdefault("reason", "") merged[test_id] = entry # A test file that no longer exists cannot be run by any shard, so pruning # it is safe even from a partial report. @@ -226,6 +247,35 @@ def self_test() -> int: assert merge({}, snapshot, "2026-07-22", lambda _t: False) == {} assert merge({}, snapshot, "2026-07-22", live) == snapshot + # Snapshot update backfills missing provenance from known_failures.json but + # preserves the snapshot's existing, more-specific explanation. + missing_issue = { + "test_gap_b": { + "status": "parity_fail", + "issue": None, + "added": "2026-01-01", + "category": "bug-open", + "reason": "specific reason", + } + } + known = { + "test_gap_b": { + "issue": "43", + "added": "2026-02-02", + "category": "gap-bisect", + "reason": "generic reason", + } + } + merged = merge( + {"test_gap_b": "parity_fail"}, + missing_issue, + "2026-07-22", + live, + known, + ) + assert merged["test_gap_b"]["issue"] == "43", merged + assert merged["test_gap_b"]["reason"] == "specific reason", merged + print("gap_snapshot self-test OK") return 0 @@ -235,6 +285,7 @@ def main() -> int: parser.add_argument("mode", nargs="?", choices=("check", "update"), default="check") parser.add_argument("--report", type=Path, default=DEFAULT_REPORT) parser.add_argument("--snapshot", type=Path, default=DEFAULT_SNAPSHOT) + parser.add_argument("--known", type=Path, default=DEFAULT_KNOWN_FAILURES) parser.add_argument("--self-test", action="store_true") args = parser.parse_args() @@ -249,7 +300,8 @@ def main() -> int: tests = load_snapshot(args.snapshot) if args.mode == "update": - merged = merge(report, tests, date.today().isoformat()) + known = load_known_failures(args.known) + merged = merge(report, tests, date.today().isoformat(), known=known) write_snapshot(args.snapshot, merged) print( f"Snapshot updated from {len(report)} run tests: " diff --git a/scripts/parity_known_failures.py b/scripts/parity_known_failures.py index a7efad0d97..fe5529922c 100644 --- a/scripts/parity_known_failures.py +++ b/scripts/parity_known_failures.py @@ -42,6 +42,11 @@ `lint`, which is a required context. It would have flagged the diagchannel entry on the day it was added. + * The same audit checks the reverse relationship: every accepted entry in + gap_snapshot.json must carry provenance and have a Linux-applicable record + here. A snapshot refresh can no longer turn a new red test into an + anonymous expected failure with no issue or explanation (#9273). + * the report path prints how many entries it actually ADJUDICATED. A gate must assert its subject was live, not merely that nothing threw. """ @@ -266,7 +271,9 @@ def audit( ) -> tuple[list[str], list[str]]: """Offline half: provenance + cross-check against the gap snapshot. - Returns (schema_problems, stale_problems). Needs no parity run, so it can + The cross-check is bidirectional: known gap entries must remain in the + snapshot, and snapshot failures must be triaged here. Returns + (schema_problems, stale_problems). Needs no parity run, so it can live on a required per-PR job while the suite that consumes this file is tag-gated. """ @@ -294,6 +301,22 @@ def test_exists(test_id: str) -> bool: f"{test_id}: test-parity/gap_snapshot.json (the generated " f"{GAP_SNAPSHOT_PLATFORM} baseline) says this test PASSES" ) + if snapshot_tests is not None: + for test_id, record in sorted(snapshot_tests.items()): + entry_problems, _ = validate_entry(test_id, record) + schema_problems.extend( + f"gap_snapshot.json {problem}" for problem in entry_problems + ) + known_record = known.get(test_id) + if known_record is None or not entry_applies( + known_record, GAP_SNAPSHOT_PLATFORM + ): + schema_problems.append( + f"{test_id}: gap_snapshot.json accepts this Linux failure, " + "but known_failures.json has no Linux-applicable entry. Add " + "one with its GitHub issue, date, category, and reason." + ) + return schema_problems, stale @@ -415,7 +438,9 @@ def entry(**overrides) -> dict: "test_gap_linux_scoped_elsewhere": entry(platforms=["windows"]), "test_parity_other_suite": entry(), } - snapshot = {"test_gap_still_broken": {"status": "parity_fail"}} + snapshot = { + "test_gap_still_broken": entry(status="parity_fail"), + } problems, stale = audit(gap_known, snapshot, test_exists=lambda _t: True) assert problems == [], problems assert len(stale) == 1 and stale[0].startswith("test_gap_fixed:"), stale @@ -428,6 +453,23 @@ def entry(**overrides) -> dict: _, stale = audit(gap_known, None, test_exists=lambda _t: True) assert stale == [], stale + # The reverse direction: a snapshot entry cannot accept a failure without + # the issue-backed known-failure record that explains it. + orphan_snapshot = dict(snapshot) + orphan_snapshot["test_gap_orphan"] = entry(status="parity_fail") + problems, _ = audit(gap_known, orphan_snapshot, test_exists=lambda _t: True) + assert ( + len(problems) == 1 and "no Linux-applicable entry" in problems[0] + ), problems + + # Nor can a matching known entry launder null provenance stored in the + # snapshot itself. + anonymous_snapshot = { + "test_gap_still_broken": entry(status="parity_fail", issue=None), + } + problems, _ = audit(gap_known, anonymous_snapshot, test_exists=lambda _t: True) + assert len(problems) == 1 and "issue must be" in problems[0], problems + print("parity_known_failures self-test OK") return 0 @@ -444,7 +486,7 @@ def run_audit(args: argparse.Namespace) -> int: return 2 if problems: - print("Malformed known_failures.json entries:", file=sys.stderr) + print("Malformed or untriaged failure metadata:", file=sys.stderr) for problem in problems: print(f" - {problem}", file=sys.stderr) return 2 @@ -459,7 +501,15 @@ def run_audit(args: argparse.Namespace) -> int: if snapshot_tests is not None else " (no gap snapshot found — cross-check skipped)" ) - print(f"known_failures.json audit OK — {entries} entries carry provenance{scope}.") + snapshot_scope = ( + f"; {len(snapshot_tests)} snapshot entries carry provenance" + if snapshot_tests is not None + else "" + ) + print( + f"known_failures.json audit OK — {entries} entries carry provenance" + f"{scope}{snapshot_scope}." + ) return 0 diff --git a/scripts/run_gap_tests.sh b/scripts/run_gap_tests.sh index f8f692650f..ac4dda619d 100755 --- a/scripts/run_gap_tests.sh +++ b/scripts/run_gap_tests.sh @@ -26,6 +26,7 @@ # Requirements: # - a Rust toolchain (the wrapped run_parity_tests.sh builds target/release/perry) # - node with --experimental-strip-types, at the .node-version pin +# - root npm dependencies installed with npm ci (package-backed Node oracles) # - jq, Python 3 (`python3` or `python`) # # Usage: scripts/run_gap_tests.sh [--shard N/M] @@ -36,6 +37,46 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" cd "$ROOT" +# #9273: the gap snapshot is meaningful only for one exact oracle environment. +# A local run used Node 26.8.1 instead of the 26.5.1 pin and had not run +# `npm ci`. Five package-backed fixtures therefore compared Node's +# ERR_MODULE_NOT_FOUND with valid Perry output and were labelled regressions. +# Refuse that experiment before spending minutes building/running the suite. +if ! command -v node >/dev/null 2>&1; then + echo "ERROR: node is required by the gap suite." >&2 + exit 2 +fi +pinned_node="$(tr -d '[:space:]' < .node-version)" +running_node="$(node --version 2>/dev/null | sed 's/^v//')" +if [[ -z "$pinned_node" || -z "$running_node" ]]; then + echo "ERROR: could not determine the Node oracle version." >&2 + exit 2 +fi +if [[ "$running_node" != "$pinned_node" ]]; then + echo "ERROR: Node oracle mismatch: running v$running_node, but .node-version pins v$pinned_node." >&2 + echo " Gap output is compared byte-for-byte, so even a patch release is a" >&2 + echo " different correctness input. Install/select the pinned version." >&2 + exit 2 +fi + +# package.json is the manifest for package-backed gap oracles. Checking the +# whole top-level install avoids a second hand-maintained package list that +# would go stale when a new fixture starts importing a dependency. npm ls +# catches both a missing node_modules tree and versions that do not satisfy the +# committed manifest; CI establishes the same state with npm ci. +if ! command -v npm >/dev/null 2>&1; then + echo "ERROR: npm is required to verify the gap suite's Node oracle dependencies." >&2 + echo " Install npm, then run: npm ci --ignore-scripts --no-audit --no-fund" >&2 + exit 2 +fi +if ! npm ls --depth=0 --silent >/dev/null 2>&1; then + echo "ERROR: root npm dependencies are missing or do not match package.json." >&2 + echo " The Node oracle must be able to import package-backed gap fixtures;" >&2 + echo " otherwise ERR_MODULE_NOT_FOUND is misreported as a Perry regression." >&2 + echo " Run: npm ci --ignore-scripts --no-audit --no-fund" >&2 + exit 2 +fi + if [[ -n "${PERRY_HOST_PLATFORM:-}" ]]; then host_platform="$PERRY_HOST_PLATFORM" else diff --git a/test-parity/README.md b/test-parity/README.md index 4a7bc603f1..85b399bbf4 100644 --- a/test-parity/README.md +++ b/test-parity/README.md @@ -40,11 +40,21 @@ revalidate (#797). Never hand-edit statuses. To accept the current state: ```bash +npm ci --ignore-scripts --no-audit --no-fund UPDATE_SNAPSHOT=1 ./scripts/run_gap_tests.sh ``` -then commit the diff. New entries land as `category: untriaged` with a null -issue — fill those in, that is the triage step the gate is asking for. +The wrapper refuses to run unless `node --version` exactly matches +`.node-version` and the root npm dependency set is installed. Both are oracle +inputs: a missing package makes Node print `ERR_MODULE_NOT_FOUND` while Perry +can still compile its compatibility implementation, manufacturing a +"regression" that no compiler change caused (#9273). + +Then commit the diff. Snapshot update copies existing triage from +`known_failures.json`. A genuinely new entry lands as an untriaged placeholder; +add its issue, date, category, and reason to both files before committing. The +required lint audit checks this relationship in both directions, so an +anonymous expected failure cannot enter through a snapshot refresh. The required CI baseline remains `gap_snapshot.json` on Linux. The gap wrapper selects `gap_snapshot.windows.json`, `gap_snapshot.macos.json`, or diff --git a/test-parity/gap_snapshot.json b/test-parity/gap_snapshot.json index 8b65de31c3..46723c64a5 100644 --- a/test-parity/gap_snapshot.json +++ b/test-parity/gap_snapshot.json @@ -3,7 +3,7 @@ "description": "Generated by scripts/gap_snapshot.py \u2014 do not hand-edit statuses. Lists every gap test that is NOT passing; a test absent from `tests` is expected to pass. CI fails on any divergence in EITHER direction (new failure, or a listed test that now passes). Regenerate with UPDATE_SNAPSHOT=1 ./scripts/run_gap_tests.sh and commit the diff.", "fields": { "status": "parity_fail | compile_fail | crash | node_fail | skipped", - "issue": "GitHub issue tracking this failure, or null when untriaged.", + "issue": "REQUIRED before commit. GitHub issue tracking this failure. Snapshot update copies it from known_failures.json when available; the offline audit rejects a null placeholder.", "added": "ISO date (YYYY-MM-DD) the test first entered the snapshot.", "category": "ci-env | module-inventory | bug-open | bug-stale | gap-categorical | gap-bisect | untriaged \u2014 see test-parity/README.md.", "reason": "Free text; most-actionable signal first." @@ -33,7 +33,7 @@ }, "test_gap_prop_plan_cache_invalidation": { "status": "parity_fail", - "issue": null, + "issue": "8271", "added": "2026-07-22", "category": "bug-open", "reason": "Root-caused 2026-08-16, no longer 'needs triage'. The test is written for SLOPPY mode (its own comments say \"writes must be dropped\" / \"silent no-op\") but runs as ESM, which is strict, so cases 3 and 4 throw instead. Node AND Perry both correctly throw; they differ only in the message. Node names the constructor -- \"of object '#'\" -- while Perry hardcodes \"'#'\" at error.rs:1726, because js_throw_type_error_immutable_write takes (kind, key_ptr, key_len) and no receiver. normalize_failure_output keeps the full error line, so the messages diverge. Real Perry parity bug, not a snapshot artifact: 14 callers, all behind the throw_immutable_write wrapper, so a receiver-aware variant fixes it without a codegen ABI change." From 4ccfd6bc3872429d57640125b3511263627c9c96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 31 Aug 2026 16:06:24 +0000 Subject: [PATCH 2/2] docs: add PR 9306 changelog fragment --- changelog.d/9306-gap-snapshot-oracle.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 changelog.d/9306-gap-snapshot-oracle.md diff --git a/changelog.d/9306-gap-snapshot-oracle.md b/changelog.d/9306-gap-snapshot-oracle.md new file mode 100644 index 0000000000..df0fdb15d8 --- /dev/null +++ b/changelog.d/9306-gap-snapshot-oracle.md @@ -0,0 +1,20 @@ +### Fixed — gap snapshots now require a valid Node oracle + +The nine regressions reported by a pristine-main gap run in #9273 were two +different artifacts. Five package-backed fixtures ran without the root +`npm ci`, so Node printed `ERR_MODULE_NOT_FOUND` while Perry produced valid +output. Four were genuine #9244 regressions that #9247 had already fixed, but +the stale run still presented them as current failures. The committed Linux +baseline remains the five failures reproduced by the pinned oracle. + +`run_gap_tests.sh` now stops before building unless `node --version` exactly +matches `.node-version` and `npm ls --depth=0` confirms the committed root +dependency graph is present. Node is the byte-for-byte reference output, so a +different patch release or missing package is a different experiment rather +than evidence of a Perry regression. Both failure paths explain how to restore +the required environment. + +Snapshot refreshes now copy existing issue/date/category/reason metadata from +`known_failures.json`, and the required offline audit checks the relationship +in both directions: every accepted Linux snapshot failure must have a +Linux-applicable, issue-backed known-failure entry. The formerly anonymous