chore: use vitest to test @lynx-js/web-worker-rpc#2038
chore: use vitest to test @lynx-js/web-worker-rpc#2038PupilTong merged 6 commits intolynx-family:mainfrom
Conversation
this is a part of lynx-family#1937
🦋 Changeset detectedLatest commit: b43f28d The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded@PupilTong has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 6 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe PR reorganizes RPC testing infrastructure by migrating test files from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR migrates the RPC testing infrastructure from Playwright browser tests to Vitest unit tests for the @lynx-js/web-worker-rpc package. This is part of a broader refactoring effort (#1937) to improve the testing setup. The migration moves tests to run directly in Node.js using node:worker_threads instead of browser-based workers, simplifying the test execution while maintaining test coverage.
- Added Vitest configuration and test infrastructure for the web-worker-rpc package
- Migrated RPC tests from Playwright to Vitest using Node.js worker threads
- Removed browser-based test files from web-tests package
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web-platform/web-worker-rpc/vitest.config.ts | New Vitest configuration for the web-worker-rpc package |
| packages/web-platform/web-worker-rpc/test/rpc.test.ts | New Vitest test file with RPC functionality tests |
| packages/web-platform/web-worker-rpc/test/worker.js | Worker implementation for testing using Node.js worker_threads |
| packages/web-platform/web-worker-rpc/test/endpoints.js | RPC endpoint definitions used by tests |
| packages/web-platform/web-worker-rpc/src/Rpc.ts | Added debug console.log statement for sync reply |
| packages/web-platform/web-worker-rpc/package.json | Added test script to run vitest |
| packages/web-platform/web-tests/tests/rpc.spec.ts | Removed Playwright-based RPC tests |
| packages/web-platform/web-tests/shell-project/rpc-test/worker.ts | Removed browser-based worker test file |
| packages/web-platform/web-tests/shell-project/rpc-test/index.ts | Removed browser-based main thread test file |
| packages/web-platform/web-tests/shell-project/rpc-test/endpoints.ts | Removed browser-based endpoint definitions |
| .changeset/warm-crabs-wait.md | Empty changeset file for this PR |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Haoyang Wang <12288479+PupilTong@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/web-platform/web-worker-rpc/vitest.config.ts (1)
4-8: Consider specifying the test environment explicitly.The configuration is minimal. Since the tests use Node.js worker_threads, consider explicitly specifying
environment: 'node'for clarity, even though it's the default.🔎 Optional enhancement
const config: UserWorkspaceConfig = defineProject({ test: { name: 'web-worker-rpc', + environment: 'node', }, });packages/web-platform/web-worker-rpc/test/rpc.test.ts (2)
69-69: Optional: Remove unnecessaryasyncmarkers from synchronous tests.Lines 69 and 83 mark tests as
asyncbut don't useawait. While not harmful, removing theasynckeyword improves clarity for synchronous test cases.🔎 Optional refactor
- test('throwError sync', async () => { + test('throwError sync', () => { expect(() => { throwErrorSyncFn(); }).toThrowError('test sync error'); }); - test('wait sync', async () => { + test('wait sync', () => { const t1 = performance.now(); waitSyncFn(1000); const t2 = performance.now();Also applies to: 83-83
105-105: Type assertion suggests incomplete type definitions.The type assertion
as unknown as ArrayBufferon Line 105 suggests the RPC type definitions may not properly reflect thathasReturnTransferendpoints unwrap{ data, transfer }and return the data directly. Consider improving the TypeScript types forRpcEndpointAsyncWithTransferto avoid requiring type assertions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.changeset/warm-crabs-wait.mdpackages/web-platform/web-tests/shell-project/rpc-test/endpoints.tspackages/web-platform/web-tests/shell-project/rpc-test/index.tspackages/web-platform/web-tests/shell-project/rpc-test/worker.tspackages/web-platform/web-tests/tests/rpc.spec.tspackages/web-platform/web-worker-rpc/package.jsonpackages/web-platform/web-worker-rpc/src/Rpc.tspackages/web-platform/web-worker-rpc/test/endpoints.jspackages/web-platform/web-worker-rpc/test/rpc.test.tspackages/web-platform/web-worker-rpc/test/worker.jspackages/web-platform/web-worker-rpc/vitest.config.ts
💤 Files with no reviewable changes (4)
- packages/web-platform/web-tests/shell-project/rpc-test/worker.ts
- packages/web-platform/web-tests/tests/rpc.spec.ts
- packages/web-platform/web-tests/shell-project/rpc-test/endpoints.ts
- packages/web-platform/web-tests/shell-project/rpc-test/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/warm-crabs-wait.md
🧠 Learnings (11)
📓 Common learnings
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1305
File: packages/react/testing-library/src/plugins/vitest.ts:4-6
Timestamp: 2025-08-11T05:59:28.530Z
Learning: In the lynx-family/lynx-stack repository, the `packages/react/testing-library` package does not have `vite` as a direct dependency. It relies on `vitest` being available from the monorepo root and accesses Vite types through re-exports from `vitest/node`. Direct imports from `vite` should not be suggested for this package.
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
📚 Learning: 2025-09-28T07:52:03.601Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts:151-154
Timestamp: 2025-09-28T07:52:03.601Z
Learning: There are two different registerUpdateDataHandler functions in the lynx-stack codebase:
1. Main thread version in packages/web-platform/web-worker-runtime/src/mainThread/crossThreadHandlers/registerUpdateDataHandler.ts takes (mainThreadRpc: Rpc, backgroundThreadRpc: Rpc, runtime: MainThreadGlobalThis)
2. Background thread version in packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/registerUpdateDataHandler.ts takes only (rpc: Rpc, tt: any)
Applied to files:
packages/web-platform/web-worker-rpc/src/Rpc.tspackages/web-platform/web-worker-rpc/test/worker.js
📚 Learning: 2025-09-28T07:52:03.601Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1837
File: packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts:151-154
Timestamp: 2025-09-28T07:52:03.601Z
Learning: There are two different registerUpdateDataHandler functions in the lynx-stack codebase:
1. Main thread version in packages/web-platform/web-worker-runtime/src/mainThread/crossThreadHandlers/registerUpdateDataHandler.ts takes (mainThreadRpc: Rpc, backgroundThreadRpc: Rpc, runtime: MainThreadGlobalThis)
2. Background thread version in packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/registerUpdateDataHandler.ts takes only (rpc: Rpc, tt: NativeTTObject)
Applied to files:
packages/web-platform/web-worker-rpc/src/Rpc.tspackages/web-platform/web-worker-rpc/test/worker.js
📚 Learning: 2025-08-29T16:57:19.221Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1235
File: packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts:24-26
Timestamp: 2025-08-29T16:57:19.221Z
Learning: In the Lynx RPC system, when createRpcEndpoint has hasReturn=true (third parameter), even if the return type is void, it returns a Promise<void> that resolves when the work on the background thread is completed. This allows proper awaiting of cross-thread operations.
Applied to files:
packages/web-platform/web-worker-rpc/test/rpc.test.tspackages/web-platform/web-worker-rpc/test/endpoints.jspackages/web-platform/web-worker-rpc/test/worker.js
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-worker-rpc/package.jsonpackages/web-platform/web-worker-rpc/vitest.config.ts
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-worker-rpc/package.jsonpackages/web-platform/web-worker-rpc/vitest.config.ts
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/warm-crabs-wait.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/warm-crabs-wait.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/warm-crabs-wait.md
📚 Learning: 2025-09-29T06:43:40.182Z
Learnt from: CR
Repo: lynx-family/lynx-stack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-29T06:43:40.182Z
Learning: Applies to .changeset/*.md : For contributions, generate and commit a Changeset describing your changes
Applied to files:
.changeset/warm-crabs-wait.md
📚 Learning: 2025-08-27T11:37:38.587Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1589
File: packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts:36-49
Timestamp: 2025-08-27T11:37:38.587Z
Learning: In web worker script loading (loadScript function), the pattern of calling fetch() before importScripts() is intentional for caching benefits, not redundant networking. The fetch() ensures HTTP caching is utilized before the synchronous importScripts() call.
Applied to files:
packages/web-platform/web-worker-rpc/test/worker.js
🧬 Code graph analysis (1)
packages/web-platform/web-worker-rpc/test/rpc.test.ts (3)
packages/web-platform/web-worker-rpc/test/worker.js (1)
rpc(28-28)packages/web-platform/web-worker-rpc/src/Rpc.ts (1)
Rpc(38-474)packages/web-platform/web-worker-rpc/test/endpoints.js (18)
addAsync(3-8)addAsync(3-8)addSync(10-16)addSync(10-16)throwError(18-23)throwError(18-23)throwErrorSync(25-25)throwErrorSync(25-25)wait(27-32)wait(27-32)waitSync(34-34)waitSync(34-34)testLazy(36-41)testLazy(36-41)addAsyncWithTransfer(43-48)addAsyncWithTransfer(43-48)changeLazyHandler(50-55)changeLazyHandler(50-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (3)
.changeset/warm-crabs-wait.md (1)
1-3: Empty changeset is appropriate for test infrastructure changes.This empty changeset satisfies CI requirements for the src file modification (console.log in Rpc.ts) without generating release notes, which is correct for internal test infrastructure changes.
packages/web-platform/web-worker-rpc/package.json (1)
31-33: LGTM! Clean test script addition.The test script follows standard conventions and properly relies on Vitest from the monorepo root.
packages/web-platform/web-worker-rpc/test/worker.js (1)
17-22: Promise.withResolvers() is compatible with the project's Node.js version requirement.The web-worker-rpc package inherits the root lynx-stack project's Node.js requirement of ^22 || ^24, which means Node.js v22 is the minimum supported version. Promise.withResolvers() is available in Node.js v22.0.0 and later, so the native API can be used directly without a fallback implementation.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #2038 will degrade performance by 7.78%Comparing Summary
Benchmarks breakdown
Footnotes
|
Web Explorer#6790 Bundle Size — 375.9KiB (0%).b43f28d(current) vs 71b9083 main#6785(baseline) Bundle metrics
|
| Current #6790 |
Baseline #6785 |
|
|---|---|---|
 Initial JS |
149.63KiB |
149.63KiB |
| Initial CSS |
32.4KiB |
32.4KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
 Modules |
231(-0.43%) |
232 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.03% |
3.03% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6790 |
Baseline #6785 |
|
|---|---|---|
| JS |
246.52KiB |
246.52KiB |
| Other |
96.98KiB |
96.98KiB |
| CSS |
32.4KiB |
32.4KiB |
Bundle analysis report Branch PupilTong:p/hw/rpc-test-isolate Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#6630 Bundle Size — 236.9KiB (0%).b43f28d(current) vs 71b9083 main#6625(baseline) Bundle metrics
|
| Current #6630 |
Baseline #6625 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
162 |
162 |
| Duplicate Modules |
65 |
65 |
| Duplicate Code |
46.74% |
46.74% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #6630 |
Baseline #6625 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.14KiB |
91.14KiB |
Bundle analysis report Branch PupilTong:p/hw/rpc-test-isolate Project dashboard
Generated by RelativeCI Documentation Report issue
this is a part of #1937
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist