Repository navigation
test: migrate legacy e2e coverage - #132
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4917b1563
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a4917b1563
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe legacy E2E suite is replaced with Rstest-based Rspack and Rsbuild tests, reusable temporary fixtures, compiler recorders, compatibility checks, watch coverage, build-output assertions, and package-contract validation. New fixtures cover basic compilation and TypeScript project references. Formatter tests now use real temporary files, worker-exit logging and RPC restart behavior gain unit coverage, and the mock-fs type dependency is removed. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
test/e2e/with-rspack/build-and-emit.test.ts (1)
39-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a default-mode case.
All four cases specify an explicit
typescript.mode; there's no case exercising the plugin's default (unsetmode) production behavior, which the PR description flags as a known coverage gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/with-rspack/build-and-emit.test.ts` around lines 39 - 126, Add a test case to the existing SolutionBuilder artifact matrix that omits `typescript.mode` and asserts the plugin’s default production artifact behavior, including expected and absent paths. Keep the case within the existing parameterized test and preserve the current explicit-mode cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/rpc/rpc-worker.ts`:
- Around line 77-90: Update the Object.defineProperties call for
remoteMethodProxy so connect, terminate, connected, and process retain the prior
enumerable, writable, and configurable property behavior from Object.assign
while preserving the live getter semantics for connected and process.
In `@test/e2e/fixtures/basic/tsconfig.json`:
- Line 5: Remove the TypeScript 6-only ignoreDeprecations setting from
test/e2e/fixtures/basic/tsconfig.json at lines 5-5 and
test/e2e/fixtures/project-references/tsconfig.base.json at lines 6-6, or replace
it with the documented TypeScript 5.0 value if suppression is required; keep
both TS 5.0 compatibility fixtures valid for typescript@5.0.4.
In `@test/e2e/package-contract.test.ts`:
- Around line 35-39: Update the pnpm invocations in the package-contract test,
including the shown pack call and the consumer install call, to use shell
execution so Windows resolves pnpm.cmd. Change the offline install flow to use
--prefer-offline or explicitly populate the pinned dependencies before
installing, ensuring the lockfile remains satisfiable without relying on cached
exact versions.
In `@test/e2e/with-rspack/tracing.test.ts`:
- Around line 32-40: Update the tracing test around getStatsMessages and
traceFiles to remove the unconditional types*.json assertion, retaining the
trace*.json event-trace assertion and existing error validation. Do not require
programmatic TypeScript trace catalog emission from
startTracing()/stopTracing().
In `@test/e2e/with-rspack/watch.test.ts`:
- Around line 260-277: Update the change-waiting logic around
recorder.waitForChangesAfter so it does not resolve on the first arbitrary
event. Collect or inspect all changes recorded after changeIndex, then assert
across the complete set that no changedFiles or deletedFiles entry ends with
package.json, ensuring the assertion validates the package/package.json edit
rather than an unrelated event.
In `@test/unit/rpc/rpc-worker.spec.ts`:
- Around line 34-37: The cleanup around worker.process should avoid awaiting a
close event after the process has already exited. In the test’s termination
flow, check exitCode/signalCode or reuse the earlier firstClose listener so only
a still-running process is awaited, while preserving cleanup for processes that
terminate during worker.terminate().
---
Nitpick comments:
In `@test/e2e/with-rspack/build-and-emit.test.ts`:
- Around line 39-126: Add a test case to the existing SolutionBuilder artifact
matrix that omits `typescript.mode` and asserts the plugin’s default production
artifact behavior, including expected and absent paths. Keep the case within the
existing parameterized test and preserve the current explicit-mode cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a9a8a01-8548-4bf3-bece-f980dfe34b70
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (73)
package.jsonsrc/rpc/rpc-worker.tssrc/typescript/worker/get-issues-worker.tssrc/typescript/worker/lib/program/watch-program.tstest/e2e-legacy/driver/listener.tstest/e2e-legacy/driver/webpack-dev-server-driver.tstest/e2e-legacy/driver/webpack-errors-extractor.tstest/e2e-legacy/fixtures/type-definitions/package.jsontest/e2e-legacy/fixtures/type-definitions/rspack.config.tstest/e2e-legacy/fixtures/type-definitions/tsconfig.jsontest/e2e-legacy/fixtures/typescript-basic/package.jsontest/e2e-legacy/fixtures/typescript-basic/rspack.config.jstest/e2e-legacy/fixtures/typescript-basic/src/authenticate.tstest/e2e-legacy/fixtures/typescript-basic/src/index.tstest/e2e-legacy/fixtures/typescript-basic/src/model/Role.tstest/e2e-legacy/fixtures/typescript-basic/src/model/User.tstest/e2e-legacy/fixtures/typescript-basic/tsconfig.jsontest/e2e-legacy/fixtures/typescript-monorepo/package.jsontest/e2e-legacy/fixtures/typescript-monorepo/packages/client/package.jsontest/e2e-legacy/fixtures/typescript-monorepo/packages/client/src/index.tstest/e2e-legacy/fixtures/typescript-monorepo/packages/client/tsconfig.jsontest/e2e-legacy/fixtures/typescript-monorepo/packages/shared/package.jsontest/e2e-legacy/fixtures/typescript-monorepo/packages/shared/src/index.tstest/e2e-legacy/fixtures/typescript-monorepo/packages/shared/src/intersect.tstest/e2e-legacy/fixtures/typescript-monorepo/packages/shared/src/subtract.tstest/e2e-legacy/fixtures/typescript-monorepo/packages/shared/tsconfig.jsontest/e2e-legacy/fixtures/typescript-monorepo/rspack.config.jstest/e2e-legacy/fixtures/typescript-monorepo/tsconfig.base.jsontest/e2e-legacy/fixtures/typescript-package/package/index.d.tstest/e2e-legacy/fixtures/typescript-package/package/index.jstest/e2e-legacy/fixtures/typescript-package/package/package.jsontest/e2e-legacy/fixtures/webpack-node-api/webpack-node-api.jstest/e2e-legacy/jest.config.jstest/e2e-legacy/jest.environment.jstest/e2e-legacy/jest.setup.tstest/e2e-legacy/out-of-memory-and-cosmiconfig.spec.tstest/e2e-legacy/tsconfig.jsontest/e2e-legacy/type-definitions.spec.tstest/e2e-legacy/type-script-config.spec.tstest/e2e-legacy/type-script-context-option.spec.tstest/e2e-legacy/type-script-formatter-option.spec.tstest/e2e-legacy/type-script-solution-builder-api.spec.tstest/e2e-legacy/type-script-tracing.spec.tstest/e2e-legacy/type-script-watch-api.spec.tstest/e2e-legacy/webpack-inclusive-watcher.spec.tstest/e2e-legacy/webpack-node-api.spec.tstest/e2e-legacy/webpack-production-build.spec.tstest/e2e/README.mdtest/e2e/fixtures/basic/src/index.tstest/e2e/fixtures/basic/src/math.tstest/e2e/fixtures/basic/tsconfig.jsontest/e2e/fixtures/project-references/packages/app/src/index.tstest/e2e/fixtures/project-references/packages/app/tsconfig.jsontest/e2e/fixtures/project-references/packages/shared/src/index.tstest/e2e/fixtures/project-references/packages/shared/tsconfig.jsontest/e2e/fixtures/project-references/tsconfig.base.jsontest/e2e/fixtures/project-references/tsconfig.jsontest/e2e/helpers/fixture.tstest/e2e/helpers/rspack.tstest/e2e/package-contract.test.tstest/e2e/package.jsontest/e2e/with-rsbuild/overlay.test.tstest/e2e/with-rspack/build-and-emit.test.tstest/e2e/with-rspack/config.test.tstest/e2e/with-rspack/loader-compat.test.tstest/e2e/with-rspack/tracing.test.tstest/e2e/with-rspack/version-compat.test.tstest/e2e/with-rspack/watch.test.tstest/unit/formatter/code-frame-formatter.spec.tstest/unit/formatter/formatter-config.spec.tstest/unit/hooks/tap-error-to-log-message.spec.tstest/unit/rpc/fixtures/restart-worker.cjstest/unit/rpc/rpc-worker.spec.ts
💤 Files with no reviewable changes (45)
- package.json
- test/e2e-legacy/fixtures/typescript-basic/src/model/User.ts
- test/e2e-legacy/fixtures/type-definitions/package.json
- test/e2e-legacy/fixtures/type-definitions/tsconfig.json
- test/e2e-legacy/fixtures/typescript-monorepo/packages/shared/src/subtract.ts
- test/e2e-legacy/webpack-inclusive-watcher.spec.ts
- test/e2e-legacy/fixtures/typescript-basic/tsconfig.json
- test/e2e-legacy/fixtures/typescript-basic/src/model/Role.ts
- test/e2e-legacy/fixtures/typescript-basic/src/index.ts
- test/e2e-legacy/fixtures/typescript-monorepo/packages/client/package.json
- test/e2e-legacy/webpack-node-api.spec.ts
- test/e2e-legacy/fixtures/webpack-node-api/webpack-node-api.js
- test/e2e-legacy/fixtures/typescript-monorepo/packages/shared/tsconfig.json
- test/e2e-legacy/fixtures/typescript-monorepo/tsconfig.base.json
- test/e2e-legacy/fixtures/typescript-monorepo/rspack.config.js
- test/e2e-legacy/fixtures/typescript-package/package/index.js
- test/e2e-legacy/fixtures/typescript-basic/package.json
- test/e2e-legacy/type-script-tracing.spec.ts
- test/e2e-legacy/jest.config.js
- test/e2e-legacy/driver/listener.ts
- test/e2e-legacy/fixtures/typescript-monorepo/packages/shared/package.json
- test/e2e-legacy/driver/webpack-errors-extractor.ts
- test/e2e-legacy/fixtures/typescript-monorepo/packages/client/tsconfig.json
- test/e2e-legacy/type-script-context-option.spec.ts
- test/e2e-legacy/fixtures/typescript-monorepo/packages/client/src/index.ts
- test/e2e-legacy/fixtures/typescript-monorepo/packages/shared/src/intersect.ts
- test/e2e-legacy/fixtures/typescript-package/package/package.json
- test/e2e-legacy/type-script-formatter-option.spec.ts
- test/e2e-legacy/fixtures/typescript-basic/src/authenticate.ts
- test/e2e-legacy/fixtures/type-definitions/rspack.config.ts
- test/e2e-legacy/fixtures/typescript-monorepo/package.json
- test/e2e-legacy/type-script-solution-builder-api.spec.ts
- test/e2e-legacy/type-definitions.spec.ts
- test/e2e-legacy/webpack-production-build.spec.ts
- test/e2e-legacy/driver/webpack-dev-server-driver.ts
- test/e2e-legacy/fixtures/typescript-basic/rspack.config.js
- test/e2e-legacy/fixtures/typescript-monorepo/packages/shared/src/index.ts
- test/e2e-legacy/out-of-memory-and-cosmiconfig.spec.ts
- test/e2e-legacy/type-script-config.spec.ts
- test/e2e-legacy/type-script-watch-api.spec.ts
- test/e2e-legacy/jest.setup.ts
- test/e2e-legacy/tsconfig.json
- src/typescript/worker/lib/program/watch-program.ts
- test/e2e-legacy/fixtures/typescript-package/package/index.d.ts
- test/e2e-legacy/jest.environment.js
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66bf7df864
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…gacy-e2e # Conflicts: # src/typescript/worker/get-issues-worker.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/e2e/package-contract.test.ts`:
- Around line 30-33: Update the package-manager execution helper around
execFileAsync so the Windows npm_execpath fallback is argument-safe: avoid
passing an argument array with shell enabled by using a safely quoted explicit
command or a resolved package-manager path. Add a regression test that unsets
npm_execpath and uses a temporary directory containing spaces, verifying the
package contract command succeeds.
In `@test/unit/rpc/rpc-worker.spec.ts`:
- Around line 5-21: Update waitForProcessExit and the surrounding RPC worker
test lifecycle to synchronize with the ChildProcess close/exit event rather than
relying solely on process.kill(pid, 0) returning ESRCH. Ensure the worker proxy
is cleared or the lifecycle completion is awaited before issuing a second
createRpcWorker call, so createRpcWorker cannot reuse a stale connected IPC
channel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a974ca01-ddaa-4007-ac0f-c881f575aa6f
📒 Files selected for processing (6)
test/e2e/package-contract.test.tstest/e2e/with-rspack/config.test.tstest/e2e/with-rspack/tracing.test.tstest/e2e/with-rspack/watch.test.tstest/unit/rpc/fixtures/restart-worker.cjstest/unit/rpc/rpc-worker.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/unit/rpc/fixtures/restart-worker.cjs
- test/e2e/with-rspack/tracing.test.ts
- test/e2e/with-rspack/config.test.ts
- test/e2e/with-rspack/watch.test.ts
Summary
This PR replaces the dormant Jest/Karton
e2e-legacysuite with active Rstest coverage built on the Rspack Node API, plus Playwright for the asynchronous Rsbuild overlay.It keeps the still-relevant behavior coverage, including configuration reloads, formatter integration, SolutionBuilder modes, tracing, watch add/change/delete/restore behavior, incremental
package.jsonhandling, production output, loader compatibility, package publishing, worker lifecycle, Rspack 1 compatibility, and the TypeScript 5.0 baseline. Obsolete Webpack-only and historical TypeScript matrices are removed.This PR is test-only relative to
main. The root-file runtime fix was split into and merged through #133.Validation
pnpm lintpnpm test— 27 files, 158 tests