Skip to content

fix(tests): unbreak class-id audit (#11691) and sloppy-this gap oracle (#11693) - #11694

Merged
proggeramlug merged 2 commits into
mainfrom
fix/11691-11693-main-reds
Sep 30, 2026
Merged

proggeramlug merged 2 commits into
mainfrom
fix/11691-11693-main-reds

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Closes #11691
Closes #11693

Two test-only fixes for reds on main. No compiler or runtime code changes, and the audit script is untouched.

#11691, class-id audit. #11678's test declared a local const CLASS_ID in object/static_shapes_tests.rs, and the audit read it as a drifted mirror of class_registry/state.rs's CLASS_ID. I renamed it to LATE_POOL_CLASS_ID. That follows the repo's existing convention: 20-odd test-local ids already carry unique *_CLASS_ID names and stay inside the audit's scope, so a test id that lands on a real reserved value is still caught. I did not weaken the audit.

  • python3 scripts/class_id_collisions.py: passes (42 ids).
  • Planted a real collision (JSX_NODE_CLASS_ID = 0xFFFF_00A1, the same value as RAW_JSON_CLASS_ID): exit 1 with COLLISION. I reverted it afterwards.
  • Put the old CLASS_ID name back in the test: it reports MIRROR DRIFT again. I reverted that too.
  • cargo test --release -p perry-runtime --lib a_class_registered_before_the_pools: 1 passed.

#11693, test_gap_sloppy_this_bound_once. Renamed .ts → .cts. The repo is "type": "module", so Node runs a .ts as strict ESM, and the oracle threw on mutate.call(1). A .cts is CommonJS, so it is sloppy in both runtimes. The repo already uses this form for sloppy-mode gap tests (test_gap_9394_…, test_gap_10509_arguments_reads_sloppy.cts, test_gap_attrs_in_shape_sloppy.cts, …). The test content is unchanged, so it still checks what #11679 intended.

  • Node 26.5.1 (/opt/node-v26.5.1-linux-x64, matching .node-version): exits 0.
  • run_parity_tests.sh --filter test_gap_sloppy_this_bound_once (PERRY_SKIP_BUILD=1 PERRY_NO_AUTO_OPTIMIZE=1, Linux x86_64, release build of this branch):
    • without the fix (the original .ts): PARITY_FAIL
    • with the fix: PASS
  • Raw stdout+stderr of Node vs the Perry binary: byte-identical (cmp).

Other checks: cargo fmt --all -- --check is clean and scripts/check_file_size.sh passes. SKIP_COMPILE_GATES=1 scripts/run_lint_gates.sh: 105 of 107 script gates pass. The 2 failures are known and not caused by this PR: "Public benchmark evidence freshness" (red on main) and "Type-check Windows runtime and stdlib" (cargo xwin is not installed on the host). I did not run the compile tier. git diff --stat was clean after the gates.

Not run: the full gap sweep, cargo test --workspace, and the compile tier of run_lint_gates.sh (known red on Linux; cargo xwin is missing on the host). I did no instruction-count A/B because no runtime or compiler code changed.

Summary by CodeRabbit

  • Tests
    • Updated test configuration so a compatibility test runs in the intended JavaScript mode.
    • Clarified a test’s local class identifier; its inputs and assertions remain unchanged.
  • User Impact
    • No changes to application features or behavior.

Ralph Küpper added 2 commits September 30, 2026 02:03
…gap oracle)

- static_shapes_tests.rs: rename the test-local CLASS_ID to
  LATE_POOL_CLASS_ID so the class-id audit no longer reads it as a
  drifted mirror of class_registry/state.rs's CLASS_ID.
- test_gap_sloppy_this_bound_once: .ts -> .cts. Under the repo's
  "type": "module" node runs a .ts as strict ESM, so the oracle threw
  on mutate.call(1); a .cts is CommonJS (sloppy) in both runtimes.
@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 84bf27c5-da15-4ba7-8183-d8d6e7044b63

📥 Commits

Reviewing files that changed from the base of the PR and between 23d5634 and ba859ba.

📒 Files selected for processing (3)
  • changelog.d/11694-fix-main-reds-class-id-sloppy-this.md
  • crates/perry-runtime/src/object/static_shapes_tests.rs
  • test-files/test_gap_sloppy_this_bound_once.cts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The static shapes test renames its local class ID constant and reformats calls without changing test inputs or assertions. The changelog records this rename and a test-file extension change.

Changes

Static shapes test update

Layer / File(s) Summary
Rename the test-local class ID
crates/perry-runtime/src/object/static_shapes_tests.rs, changelog.d/11694-fix-main-reds-class-id-sloppy-this.md
The test uses LATE_POOL_CLASS_ID instead of CLASS_ID and reformats the key-array and shape-ID calls. The changelog records the rename and a test-file extension change.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to ba859

This change updates test fixtures and documentation without changing runtime behavior. The parity runner includes the renamed test, and no actionable merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies both test-only fixes: the class-ID audit issue and the sloppy-this gap oracle issue.
Description check ✅ Passed The description provides the summary, linked issues, concrete changes, verification results, known gate failures, and tests not run. It does not reproduce the template headings or checklist, but the r…
Linked Issues check ✅ Passed The PR addresses both direct coding objectives. For #11691, it renames the test-local CLASS_ID to LATE_POOL_CLASS_ID and leaves scripts/class_id_collisions.py unchanged. The reported audit resul…
Out of Scope Changes check ✅ Passed The changes stay within the linked issue scope. They modify one test-local constant, rename one parity test file, and add a changelog entry that documents these fixes. No compiler or runtime code chan…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant