Skip to content

fix(util): handle diff edge cases - #40166

Closed
deepshekhardas wants to merge 4 commits into
oven-sh:mainfrom
deepshekhardas:fix-39728-util-diff
Closed

deepshekhardas wants to merge 4 commits into
oven-sh:mainfrom
deepshekhardas:fix-39728-util-diff

Conversation

@deepshekhardas

Copy link
Copy Markdown

Fixes #39728

Handle util.diff edge cases.

deepshekhardas added 4 commits August 20, 2026 13:37
When toMatchInlineSnapshot() is the last expression of a test function body, JavaScriptCore applies tail-call optimization and elides the caller frame, so get_caller_src_loc() returns an empty location and the snapshot writer reports 'called from file: '.

expect() itself is never in tail position (its result feeds the matcher member access), so capture its caller src loc when the Expect is created and fall back to it when the matcher walk finds nothing. The matcher call site is then relocated by reading the test file and finding the fn_name( call at/after the expect() position.

@claude claude Bot left a comment

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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Your included review limit has been reached.

You’re in a promotional period — use the checkbox below to run this review for free:

  • Run review for free

On-demand reviews are free for the next 29 days. After that, they cost $0.25 per reviewed file.

How can I continue?

Run this review now using the option above, or comment @coderabbitai review --use-credits.

You can also wait for the limit to reset (next review available in 36 minutes), then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 54078f30-a1d7-4437-8938-56a53caa0cbe

📥 Commits

Reviewing files that changed from the base of the PR and between 8eb5b6e and ff02959.

📒 Files selected for processing (10)
  • src/bundler/bundle_v2.rs
  • src/bundler/linker.rs
  • src/js/node/util.ts
  • src/jsc/bindings/BunProcess.cpp
  • src/jsc/lib.rs
  • src/runtime/test_runner/expect.rs
  • test/cli/test/bun-test.test.ts
  • test/js/bun/resolve/resolve-error.test.ts
  • test/js/node/process/process-sourcemaps-enabled.test.ts
  • test/js/node/util/util-diff.test.ts

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.

@robobun

robobun commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Thank you for the PR. This branch is the same commit (ff02959) as #39749. #39749 was closed on August 20 in favor of #39731, and #39731 is still open for #39728. To keep one PR per issue in the review queue, I am closing this one as a duplicate of #39731.

As noted on #39749: both PRs port the same Myers diff from Node, and the output of #39731 matched Node 26.3.0 on 3593 random string and array pairs. The util-diff.test.ts from this branch passes against #39731 without changes.

This branch also still carries the commits of #40157, #39744 and #39748, so it could not merge on its own. Please branch each change from main so that each PR contains only its own diff.

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.

Builtin surface is missing 16 exports and 2 modules that Node 24.11 has, while process.versions.node reports 26.3.0

2 participants