Skip to content

test(vitest): adapt happy-dom OffscreenCanvas canvas mocks - #17336

Merged
nwparker merged 1 commit into
mainfrom
nwparker/fix-vitest-happy-dom-offscreen-canvas
Aug 30, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/fix-vitest-happy-dom-offscreen-canvas

Conversation

@nwparker

@nwparker nwparker commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​67 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​67
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​60 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​60

Root cause

happy-dom@20.11.8 now exposes OffscreenCanvas, but without a configured canvas adapter its getContext("2d") returns null. xterm intentionally prefers that API and throws while creating its WidthCache, so otherwise-unrelated unit shards fail before reaching their assertions. The same failure reproduces on the dependency upgrade PR #17330.

Fix

Install one idempotent Vitest compatibility setup: when an OffscreenCanvas 2D context is adapter-less, delegate to the existing HTMLCanvasElement.prototype.getContext test double. Real adapters and non-2D contexts are returned unchanged. Production/browser code is untouched.

Regression proof

  • Added compatibility tests for adapter-less 2D delegation and non-2D isolation.
  • Added an xterm Terminal.open() regression proving the current happy-dom setup can construct the DOM renderer.
  • Verified on happy-dom 20.9.0 and an isolated origin/main environment with happy-dom 20.11.8 + Vitest 4.1.11.

Local verification

  • Vitest compatibility + xterm regression: 3/3 passed on 20.11.8 (and 20.9.0).
  • Existing xterm placeholder-mask focused suite: 13/13 passed.
  • oxlint --deny-warnings, oxfmt --check, and git diff --check: passed.

Readiness / elegance

  • Test-only, no IPC/RPC, wire, SSH, mobile, persistence, packaging, or production rendering changes.
  • The shim addresses the dependency contract mismatch at the Vitest environment boundary; it does not suppress xterm errors or alter xterm production code.
  • It preserves available canvas implementations, is bounded and idempotent, and reuses the existing HTML canvas doubles.
  • Native/Electron verification is not applicable to this test-environment-only change.

Closes the post-#17330 test baseline regression affecting #17169, #17170, and #17172.

@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a test-only compatibility layer that lets adapter-less happy-dom OffscreenCanvas 2D requests reuse the existing HTML canvas mock.

  • Registers the compatibility layer in the shared Vitest setup.
  • Preserves native or adapter-provided contexts and leaves non-2D requests unchanged.
  • Adds focused delegation, isolation, and xterm rendering regression tests.

Confidence Score: 5/5

The PR appears safe to merge with no actionable correctness or security issues identified.

The compatibility layer is limited to test environments, safely no-ops when required DOM globals are absent, preserves existing OffscreenCanvas contexts, and delegates only null 2D results to the established HTML canvas mock.

Important Files Changed

Filename Overview
config/scripts/happy-dom-offscreen-canvas.ts Adds a guarded, idempotent OffscreenCanvas 2D fallback that preserves available adapters and unsupported context behavior.
config/scripts/happy-dom-offscreen-canvas.test.ts Covers adapter-less delegation, non-2D isolation, and successful xterm DOM renderer initialization.
config/vitest.config.ts Loads the compatibility shim through the existing shared Vitest setup sequence.

Sequence Diagram

sequenceDiagram
  participant X as xterm
  participant O as OffscreenCanvas
  participant C as Compatibility wrapper
  participant H as HTMLCanvasElement mock
  X->>O: getContext("2d")
  O->>C: adapter result
  alt Existing adapter returns a context
    C-->>X: existing context
  else Adapter returns null
    C->>H: getContext("2d")
    H-->>C: test context
    C-->>X: delegated context
  end
Loading

Reviews (1): Last reviewed commit: "test: bridge happy-dom OffscreenCanvas c..." | Re-trigger Greptile

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ae634c99-ec64-4d9e-9e5a-43e2e96066e4

📥 Commits

Reviewing files that changed from the base of the PR and between 558f4e0 and 4a237bd.

📒 Files selected for processing (3)
  • config/scripts/happy-dom-offscreen-canvas.test.ts
  • config/scripts/happy-dom-offscreen-canvas.ts
  • config/vitest.config.ts

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


📝 Walkthrough

Walkthrough

Adds a happy-dom compatibility shim for OffscreenCanvas. The shim delegates failed 2D context requests to an HTML canvas context and leaves unsupported contexts unchanged. Vitest loads the shim during setup. Tests cover delegation, missing globals, WebGL behavior, cleanup, and xterm terminal rendering.

Merge Risk: ⚪ Minimal · up to 4a237

This change adds a localized test-environment compatibility shim so xterm tests can construct canvas-backed renderers under newer happy-dom versions without changing production behavior. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the root cause, fix, regression tests, verification, and scope. However, it does not follow the required template and omits required information, including an explicit Linked … Rewrite the description using all template headings. Add the issue reference in the Linked Issue section, provide visual proof or write exactly N/A with a reason, complete the Testing checklist with platforms tested, complete AI Disclosure …
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adapting Vitest happy-dom OffscreenCanvas canvas mocks.
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 3 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 root cause, fix, regression tests, verification, and scope. However, it does not follow the required template and omits required information, including an explicit Linked Issue entry, Visual Proof or the exact N/A statement, testing checklist and platform details, AI Disclosure, Review, Agent skill upstream boundary, Notes, and the final Checklist.

Resolution

Rewrite the description using all template headings. Add the issue reference in the Linked Issue section, provide visual proof or write exactly N/A with a reason, complete the Testing checklist with platforms tested, complete AI Disclosure when applicable, complete Review, Agent skill upstream boundary, Notes, and Checklist sections, and retain the existing root cause, fix, regression proof, and verification details.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

@nwparker

Copy link
Copy Markdown
Contributor Author

CI result for head 4a237bd2de49: all eight Node 24 test shards, typecheck, guards, and review checks pass. The only red is static analysis (and its aggregate verify) on the pre-existing src/renderer/src/components/native-chat/NativeChatView.tsx:461 max-lines violation (403 code lines), reproduced on origin/main and present in merged #16883. No changed file is implicated. I am keeping this PR test-only and tracking the required behavior-preserving component split separately; no lint disable or max-lines baseline exception is being added.

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

  • OffscreenCanvas shim — config/scripts/happy-dom-offscreen-canvas.ts patches OffscreenCanvas.prototype.getContext so an adapter-less 2d context delegates to the existing HTMLCanvasElement.prototype.getContext double; non-2d and real-adapter results pass through unchanged. Mirrors the established happy-dom-mutation-observer-retention.ts pattern (idempotency symbol, self-install, no-op when globals are absent).
  • Setup wiring — config/vitest.config.ts registers the shim as the first setupFiles entry.
  • Regression coverage — config/scripts/happy-dom-offscreen-canvas.test.ts proves 2d delegation, non-2d isolation, and a real Terminal.open() succeeding against the current happy-dom DOM renderer.

The delegation reads HTMLCanvasElement.prototype.getContext at call time, so per-test vi.spyOn(...).mockReturnValue(...) doubles are picked up correctly, and the shim is a no-op in the default node environment (no OffscreenCanvas/HTMLCanvasElement/document). All three assertions would fail if the shim regressed, so the coverage is real rather than theatre. No production, IPC, wire, or rendering surface is touched.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@nwparker

Copy link
Copy Markdown
Contributor Author

CI baseline note: all eight Node 24 test shards, typecheck, guards, LoC, Pullfrog, and Greptile pass. The only red is static analysis (and its aggregate verify), caused by the pre-existing NativeChatView max-lines violation at line 461; the same failure reproduces on origin/main and is isolated in #17339. The OffscreenCanvas compatibility diff itself has no lint findings and is ready for direct merge; #17339 clears the independent lint baseline separately.

@nwparker
nwparker merged commit 9e993dd into main Aug 30, 2026
26 of 28 checks passed
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