Skip to content

test(js): verify worklet modules without Vite build setup - #7824

Merged
moq-bot[bot] merged 8 commits into
mainfrom
quest/m0/test-flakes-2/js-worklet-build
Nov 4, 2026
Merged

moq-bot[bot] merged 8 commits into
mainfrom
quest/m0/test-flakes-2/js-worklet-build

Conversation

@kixelated

@kixelated kixelated commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The worklet library-build test spends Bun's five-second test budget on a complete Vite build and times out under CPU contention. This completes quest/m0/test-flakes-2/js-worklet-build.md.

Approach

  • Test the plugin's production load hooks with its real in-process compiler in the fast suite. Check the lazy blob, hosted filename, identical embedded and hosted code, and absence of child processes.
  • Move full Vite output and build-lifecycle assertions into the existing nightly just js worklet-matrix, which builds and loads all three real worklets through Vite, webpack, esbuild and Bun, including strict CSP. Assert no child remains before starting consumer bundlers or Chromium.
  • Remove the completed quest and its parent link.

Impact

  • Public API: none.
  • Wire: none.
  • Production code and Bun timeouts are unchanged. Full Vite chunking, emitted-file placement, rendered import.meta.url, and completed-build lifecycle assertions now run nightly. Fast tests retain real compilation, lazy/hosted module checks and the no-child-process contract.

Alternatives and choices

Chose the quest Plan's cheaper verification level. Profiling attributes most time to Vite's surrounding build setup and bundling, so deduplicating the plugin's two compilations would leave the main source of the test's load sensitivity. A native outer Rolldown bundle still timed out once in three runs under the same load, so the fast check invokes the hooks directly. No production caching or benchmark machinery is added. The nightly matrix keeps the production library minification and consumer bundler modes.

Checked all current open PR changed files before claiming. None changes the plugin, its unit test, or the nightly worklet matrix. moq-dev/moq#7821 fixes separate exact Bun path selection; focused runs here use ./js/common/worklet.test.ts without importing its branch.

Validation

  • Before editing, ran the real Vite build three times without controlled load, then three times with seven CPU competitors on one CPU. Fresh-process import cost: 86–119 ms unloaded and 0.98–2.01 s loaded. Build cost: 231–318 ms unloaded and 2.31–2.86 s loaded. The two plugin compilation hooks total 0.42–0.59 s in the loaded builds; no child survives a completed build.
  • Reproduced all three library-test timeouts with fifteen CPU competitors on one CPU: 5013, 5039 and 5064 ms. Separate complete builds at that contention took 3.93–5.06 s, with 1.21–1.29 s in the two compile hooks and no surviving child. One dev-server timeout followed the first aborted build; the next two dev-server cases passed.
  • After the change, all three fresh-process loaded repetitions pass: the production-hook test takes 1.79–2.05 s and the unchanged dev-server test takes 2.13–2.50 s, at the same fifteen-competitor contention. There are no retries, timeout changes or surviving children.
  • just js worklet-matrix: passed on a9122739938a3a55b1f26903e850fefde74a9b97 with default production library minification. The real Vite library emits three hosted scripts and three lazy blob chunks, contains no import.meta.url, and leaves no child. All four consumer bundlers pass default and strict-CSP hosted modes; all four strict-CSP blob controls fail as expected.
  • Focused unit tests and a targeted type check with the repository's compiler settings and Bun types pass. Expensive and loaded runs use the pinned Nix shell and quest gate.
  • just ci: passed on a9122739938a3a55b1f26903e850fefde74a9b97, including the complete JS workspace check/build/API reports and tests, tooling checks/tests, lint and quest checks. git diff --check and quest check --base origin/main also pass.

Review

mechanical fix by quest land watch: merged main at acfb9b3b7058d5065eab5f85dd98b2387ae6c21e with the quest merge driver (head 2cf0b44)

mechanical fix by quest land watch: merged main at 4d85daa4a1e7c81915b27d658ca863569a9e8b87 with the quest merge driver (head d177e0d)

mechanical fix by quest land watch: merged main at 781e238a013d2dc53fa384583b0e4bf3a6280073 with the quest merge driver (head 6458d96)

Claude Code via quest review --base origin/main, Claude Opus 5.5, medium effort, round 2 reviewed a9122739938a3a55b1f26903e850fefde74a9b97: no findings.

Round 1: fixed the unnecessary minification override and stale unit-test comment. Refuted the two approach findings with the measured build/compile costs, the loaded native-bundle experiment, the Plan's permitted cheaper verification level, and explicit disclosure that full Vite output/lifecycle assertions run nightly. The final matrix with production minification and just ci both pass.

Follow-ups

None. No maintainer decisions remain.

(Written by GPT-6.1 Sol)

kixelated and others added 5 commits November 3, 2026 22:07
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
@kixelated kixelated changed the title The JS worklet library-build test survives load test(js): verify worklet modules without Vite build setup Nov 4, 2026
@kixelated
kixelated marked this pull request as ready for review November 4, 2026 07:08
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The worklet unit test now calls the plugin hooks directly instead of running a Vite library build. The matrix test continues to build with Vite and adds checks for generated assets and JavaScript. The related test-flake task entry and plan have been removed.

Merge Risk: Minimal · up to 2cf0b

The fast test avoids a full Vite build while retaining production-hook checks, and the matrix retains generated-build validation. No actionable merge-blocking issue was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: verifying worklet modules without the full Vite build setup.
Description check Passed The description directly explains the timeout problem, the testing approach, the nightly matrix changes, and validation results for this changeset.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
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: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Finishing Touches
Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 21f9c3fc-0b29-42e9-8937-cb57ec945ded
Commits

Reviewing files that changed from the base of the PR and between d177e0d and 2cf0b44.

Files selected for processing: 1
  • quest/m0/test-flakes-2/README.md
Files skipped from review as they are similar to previous changes: 1
  • quest/m0/test-flakes-2/README.md

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

@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

  • Fast worklet test drops the full Vite build — js/common/worklet.test.ts now invokes the plugin's real configResolved/load hooks directly to check the emitted assets/render.js, the hosted new URL("render.js", base) entry, the lazy ?worklet-blob import, identical embedded/hosted code, and the no-child contract. Ran it locally: passes in ~21 ms.
  • Nightly matrix gains the end-to-end assertions — test/worklet/matrix.ts now verifies, after the real Vite library build of all three worklets, that no child process remains and that the emitted assets, blob chunks, lazy imports, and hosted URLs are exactly as expected. Reproduced the build plus all new assertions locally: passes.
  • Quest closed — removes quest/m0/test-flakes-2/js-worklet-build.md and its parent link.

The removed full-build assertions are fully reproduced nightly, and the remaining integration gaps (resolveId, real Vite output) are explicitly covered there. No production or wire change.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed head a912273. No actionable issue found in the test restructuring. The fast test exercises the production hooks with the in-process compiler, while the Nightly matrix retains the full Vite output, lazy-loading, hosted-asset and child-lifecycle assertions.

Verification: GitHub-only static review of the plugin, tests and Nightly wiring. I did not run the load reproducer, matrix or CI; those results are author-reported.

(Written by review (OpenAI))

@kixelated
kixelated added this pull request to the merge queue Nov 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 4, 2026
@moq-bot

moq-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

quest land watch: the merge queue removed this pull request at a912273 because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

@moq-bot
moq-bot Bot enabled auto-merge November 4, 2026 14:38
@moq-bot

moq-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

quest land watch: armed auto-merge on this pull request at 6458d96, so it joins the back of the merge queue once its required checks pass, with main at 781e238 merged in by the quest merge driver.

(written by quest land watch)

@moq-bot
moq-bot Bot added this pull request to the merge queue Nov 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 4, 2026
@moq-bot

moq-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

quest land watch: the merge queue removed this pull request at 6458d96 because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

@moq-bot
moq-bot Bot enabled auto-merge November 4, 2026 14:42
@moq-bot

moq-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

quest land watch: armed auto-merge on this pull request at d177e0d, so it joins the back of the merge queue once its required checks pass, with main at 4d85daa merged in by the quest merge driver.

(written by quest land watch)

@moq-bot
moq-bot Bot added this pull request to the merge queue Nov 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 4, 2026
@moq-bot

moq-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

quest land watch: the merge queue removed this pull request at d177e0d because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

@moq-bot
moq-bot Bot enabled auto-merge November 4, 2026 14:53
@moq-bot

moq-bot Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

quest land watch: armed auto-merge on this pull request at 2cf0b44, so it joins the back of the merge queue once its required checks pass, with main at acfb9b3 merged in by the quest merge driver.

(written by quest land watch)

@moq-bot
moq-bot Bot added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit fa2dd81 Nov 4, 2026
8 of 10 checks passed
@moq-bot
moq-bot Bot deleted the quest/m0/test-flakes-2/js-worklet-build branch November 4, 2026 14:59
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