Skip to content

test(errors): align constructor positions and select stack regressions - #105

Merged
steipete merged 1 commit into
mainfrom
claude/w138-fork-source-positions
Oct 4, 2026
Merged

steipete merged 1 commit into
mainfrom
claude/w138-fork-source-positions

Conversation

@steipete

@steipete steipete commented Oct 4, 2026 •

Copy link
Copy Markdown

#104 correctly moves constructor stack positions from the opening parenthesis after Error to the new keyword, but left five stale expectations in the WebKit upgrade and VM sourceURL tests. Both files were omitted from that PR's CI selection because they were only selected for WebKit pin changes.

Update the exact constructor coordinates and derive the generated line-table expectations from new Error(...). Keep all cases, exact comparisons, offset checks, line endings, and cached-bytecode checks. Select both suites on native CallSite, ErrorStack, FormatStackTraceForJS, and ZigSourceProvider changes, with selector regression coverage for Linux and macOS. Runtime code and the WebKit pin are unchanged.

The complete Linux artifact bisect identifies 79bce2c (#104) as the first failing commit. Both files pass (22 + 3 tests) at 61e5bf3, c999d9c, 1ac7fda, 72f794a, 9d0fb41, 65023a5, 32fd1a2, 02a67c2, 372d569, and 9297af6. #104 fails exactly 4 + 1 cases. Each predecessor artifact's CI merge tree was verified identical to its squash tree; #104 used its main CI artifact.

Node 24.21.0 confirms every changed coordinate:

Case Old expectation Node and #104
VM constructor 6:37 6:28
First-line VM constructor with offsets 11:21 11:12
Later-line VM constructor with offsets 12:16 12:7
Short eval constructor 2:19 2:10
VM sourceURL constructor hellohello.js:2:16 hellohello.js:2:7

The randomized line-table corpus additionally matches Node on all 512 positions across 8-/16-bit sources, line-ending variants, and parsed/cached bytecode. Existing ordinary-call differences from Node remain unchanged.

Validation on Linux, using the unchanged #104 binary with the corrected tests: 444 pass, 0 fail across the WebKit file (22), VM sourceURL (3), CI selector (14), CallSite (86), and VM (319). VM retains 3 existing skips and 60 todos. Prettier and diff checks pass. The required branch P2 review is scoped-clean. Both fork CI lanes are green on exact head 3e0a8df7b53bb683cb7f552e7ab95c76c8467653 in run 37197000130. Downloaded reports confirm both source-position files were selected and passed on Linux (22 selected files) and macOS (18 selected files).

macOS required two same-head retries: attempt 1 failed the unchanged VM wall-clock timeout test; attempt 2 passed that test but failed the unchanged script-leak RSS limit at about 202 MiB against 200 MiB. Each failed test passed in the other attempt, and attempt 3 passed all 18 selected test files. Both source-position files passed in every attempt. No assertions, memory limits, selections, or runtime code were changed in response. The separate duplicate-review bot failed before review because its Anthropic credentials are not configured in the fork.

Upstream status: oven-sh#35179, oven-sh#37396, and oven-sh#44559 remain open; #104 already adapts the native correction. This follow-up needs no additional runtime port.

@steipete
steipete merged commit b83b544 into main Oct 4, 2026
10 of 13 checks passed
@steipete
steipete deleted the claude/w138-fork-source-positions branch October 4, 2026 11:41
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