Skip to content

test(9087): class toString reports source identity after #9465 - #9477

Merged
proggeramlug merged 1 commit into
mainfrom
fix/9087-class-tostring-source
Sep 2, 2026
Merged

test(9087): class toString reports source identity after #9465#9477
proggeramlug merged 1 commit into
mainfrom
fix/9087-class-tostring-source

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

cargo-test-perry (2/8) fails on main:

test class_refs_use_string_concatenation_for_dynamic_add ... FAILED
  left:  "string class K { static m(): number { return 1; } }1\n…"
  right: "string function K() { [native code] }1\n…"

The code is right; the test expectation is stale. e86a2611cf ("class name/toString/inspect report source identity", from #9465) deliberately changed String(SomeClass) from function K() { [native code] } to the class's source text — which is what node does:

$ node -e 'class K { static m() { return 1; } }; console.log("string " + K + "1")'
string class K { static m() { return 1; } }1

issue_9087_class_ref_add.rs was not updated alongside it. This updates the three affected lines to the new (more spec-correct) output, with the node verification recorded inline so the next reader does not have to re-derive it.

Perry reports the TypeScript source it compiled, so the : number annotation is retained in the expected string — that difference is noted in the comment.

Verified locally: cargo test -p perry --test issue_9087_class_ref_add1 passed; 0 failed.

Blocking: cargo-test-perry is in full-suite-gate's needs, so this stops any release cut.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected string conversion for declared classes used in dynamic concatenation.
    • Class values now stringify to their TypeScript source identity instead of a native-function representation.
    • Preserved existing numeric conversion behavior for valueOf cases.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: c1dd4d27-1fd3-4875-9ef9-b01460b35940

📥 Commits

Reviewing files that changed from the base of the PR and between 0a1c137 and d2768ca.

📒 Files selected for processing (1)
  • crates/perry/tests/issue_9087_class_ref_add.rs

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


📝 Walkthrough

Walkthrough

The regression test now expects class references to stringify to their TypeScript source identity during dynamic + concatenation. The valueOf result remains 42.

Changes

Class Reference Stringification

Layer / File(s) Summary
Update regression expectations
crates/perry/tests/issue_9087_class_ref_add.rs
The test updates expected output for right-side, left-side, and accumulator concatenation. The valueOf expectation remains unchanged.

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

Merge Risk: ⚪ Minimal · up to d2768

This change updates stale test expectations to match the existing class stringification behavior and does not alter production code; no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the test update and its cause: class toString behavior changed after #9465.
Description check ✅ Passed The description explains the stale expectation, lists the affected behavior, records Node.js verification, and provides the local test command and result. It does not use all template headings or incl…
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.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the stale expectation, lists the affected behavior, records Node.js verification, and provides the local test command and result. It does not use all template headings or include the checklist, but it contains the critical change and verification details.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/9087-class-tostring-source

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.

@proggeramlug
proggeramlug merged commit 1659211 into main Sep 2, 2026
28 of 29 checks passed
@proggeramlug
proggeramlug deleted the fix/9087-class-tostring-source branch September 2, 2026 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant