chore(workflow): migrate unit test runner from vitest to rstest#2698
chore(workflow): migrate unit test runner from vitest to rstest#2698fi3ework wants to merge 1 commit into
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: d9b3ffaae3
ℹ️ 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".
| coverage: createCoverageConfig(__dirname), | ||
| testEnvironment: 'node', | ||
| include: ['tests/**/*.test.ts'], |
There was a problem hiding this comment.
This node-target config is the only Rstest config for suites importing @midscene/shared/img that does not apply photonExternal. The evaluation tests import saveBase64Image / imageInfoOfBase64, which pulls in shared/src/img/get-photon and its @silvia-odwyer/photon dynamic import; without output.externals, Rspack tries to bundle that native/wasm package and the run can fail before tests start. Add output: { externals: photonExternal } here as in the other node test configs.
Useful? React with 👍 / 👎.
| coverage: createCoverageConfig(__dirname), | ||
| testEnvironment: 'node', | ||
| include: ['tests/**/*.test.ts'], |
There was a problem hiding this comment.
The deleted Vitest config loaded ../../.env, but this replacement config no longer runs the repo-level dotenv setup. When running the package scripts such as pnpm --filter @midscene/evaluation evaluate:locator with model credentials only in the workspace .env, the Rstest worker will not inherit MIDSCENE_MODEL_*; the test files' own dotenv.config() resolves from packages/evaluation, so globalModelConfigManager.getModelConfig() can fail with missing model configuration. Add the same globalSetup used by the other AI-backed Rstest configs (Rstest documents that globalSetup env mutations are propagated to node workers: https://rstest.rs/config/test/global-setup).
Useful? React with 👍 / 👎.
Replace vitest with @rstest/core across all packages and apps. Each project gains an rstest.config.ts; shared boilerplate (coverage, dotenv, jsdom storage polyfill, style stubs, photon externals, __VERSION__ define) is factored into scripts/rstest-*.ts. Two rstest behaviors required workarounds, each tracked upstream: - apps/studio launch-electron: rstest mocks are an rspack build-time transform, so import(variable) escapes them and would spawn a real Electron process. Use string-literal imports. (web-infra-dev/rstest#1454) - apps/studio updater-handlers: rstest externalizes node-env deps by default and still evaluates an externalized module even when mocked, so the real electron-updater crashed at import. Bundle all deps via output.bundleDependencies. (web-infra-dev/rstest#1456) core merge-browser-parse: replace the brittle absolute 15MB cap on the merged report with a guard relative to a single report, so it tracks the report-template size instead of breaking as the viewer bundle grows. Validation: pnpm run lint; full test:coverage (13 projects, 2328 tests) all green. Claude-Session: https://claude.ai/code/session_011FdpdFgXxUwWgRxpALzfoE
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95f0b74ba5
ℹ️ 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".
| }, | ||
| "devDependencies": { | ||
| "@rslib/core": "^0.18.3", | ||
| "@rstest/core": "0.10.6", |
There was a problem hiding this comment.
This migrated package now depends on @rstest/core@0.10.6, but the lockfile for that package declares engines: {node: ^20.19.0 || >=22.12.0} while the workspace still advertises Node >=18.19.0. On a documented Node 18.x development environment, pnpm --filter @midscene/core test now invokes a runner outside the supported engine range; either bump the repo's Node requirement everywhere or use a runner version compatible with Node 18.
Useful? React with 👍 / 👎.
| import { getModelRuntime } from '@/ai-model/models'; | ||
| import { globalModelConfigManager } from '@midscene/shared/env'; | ||
| import { expect, test, vi } from 'vitest'; | ||
| import { expect, rs, test } from '@rstest/core'; |
There was a problem hiding this comment.
After switching this file to @rstest/core, the existing test(..., { timeout }, async () => ...) calls below no longer match Rstest's documented signature, which takes the callback second and options third. When AITEST=true runs the core AI tests, these cases will be registered with the options object in the callback slot instead of exercising AiLocate*; move the async function to the second argument and the timeout object to the third (or rely on rs.setConfig).
Useful? React with 👍 / 👎.
| export default function setup() { | ||
| dotenv.config({ | ||
| path: path.resolve(__dirname, '..', '.env'), | ||
| override: true, |
There was a problem hiding this comment.
With override: true, this global setup replaces values already provided by the shell or CI whenever a workspace .env exists. That regresses the previous non-core Vitest configs where MIDSCENE_MODEL_* or related credentials passed inline (for example to compare models locally) won over .env; AI tests will silently use the stale file values inside Rstest workers. Drop the override unless a specific suite needs file values to take precedence.
Useful? React with 👍 / 👎.
1 participant
What
Migrate the repository's own unit-test tooling from vitest to rstest (
@rstest/core) across every package and app.rstest.config.ts(replacingvitest.config.ts).@silvia-odwyer/photonexternals, and the__VERSION__define — is factored intoscripts/rstest-*.tsinstead of being copy-pasted per package.rstest behaviors hit during migration
Two upstream issues were filed; both are worked around in-tree with a
TODOpointing at the issue:apps/studiolaunch-electron— rstest'srs.mock()is injected via an rspack build-time transform, so a dynamicimport(variable)escapes it and loaded the real launcher, spawning a real Electron process. Switched to statically-analyzable string-literal imports. → [Bug]: rs.mock is bypassed for modules loaded via dynamic import() with a non-literal specifier rstest#1454apps/studioupdater-handlers— rstest externalizes node-environment deps by default and still evaluates an externalized module even when it isrs.mock()-ed, so the realelectron-updaterran its init and crashed (require('electron').appis undefined outside the Electron runtime). Bundled all deps viaoutput.bundleDependencies: true. → [Bug]: rs.mock() does not prevent evaluation of an externalized dependency (real module still runs) rstest#1456Other fix
coremerge-browser-parse— the test capped the merged report at an absolute 15 MB; the self-contained report viewer template has since grown past that. Replaced it with a guard relative to a single source report (< 1.5×), which still catches real bloat (inlined screenshots / duplicated template ≈ 2×) without breaking each time the viewer bundle grows.Validation
pnpm run lint(biome check + check-dependency-version) — passnx run-many --target=build, 33 projects) — passtest:coverage(13 projects, 2328 tests) — all greenhttps://claude.ai/code/session_011FdpdFgXxUwWgRxpALzfoE