Skip to content

feat(test): switch wasm integration tests to vitest - #1302

Draft
quexten wants to merge 2 commits into
mainfrom
km/integration-vitest
Draft

feat(test): switch wasm integration tests to vitest#1302
quexten wants to merge 2 commits into
mainfrom
km/integration-vitest

Conversation

@quexten

@quexten quexten commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Switches our wasm integration tests from jest to vitest. Vitest is more modern and way faster, at basically no maintenance cost.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🔍 SDK Breaking Change Detection

SDK Version: km/integration-vitest (47190ff)

⚠️ If breaking changes are detected, a corresponding pull request addressing them must be ready for merge in the affected client repository.

Client Status Details
typescript ✅ No breaking changes detected Compilation passed with new SDK version - View Details
android ✅ No breaking changes detected Compilation passed with new SDK version - View Details

Breaking change detection uses the build of the SDK from this branch, including any incompatibities pre-existing on or merged into this branch. Check the workflow logs to confirm.
Results update as workflows complete.

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.81%. Comparing base (a91e406) to head (935426e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1302   +/-   ##
=======================================
  Coverage   85.81%   85.81%           
=======================================
  Files         490      490           
  Lines       70590    70590           
=======================================
  Hits        60574    60574           
  Misses      10016    10016           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@quexten quexten added the ai-review Request a Claude code review label Jul 28, 2026
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the migration of the WASM integration test suite from Jest/ts-jest to Vitest: removal of jest.config.js, the new vitest.config.ts, the package.json script/dependency swap, and the tsconfig.json types change. globals: true plus include: ["**/*.test.ts"] preserve the previous discovery and execution semantics, no test file uses Jest-only APIs, no .js-extension imports depend on the dropped moduleNameMapper, and no Jest references remain anywhere in the repo. The only change since the last review pass is a merge from main, so the two previously posted findings still apply and were not re-posted.

Code Review Details
  • ⚠️ : New dependency vitest added without a referenced AppSec approval signal
    • crates/bitwarden-wasm-internal/integration-tests/package.json:15
  • ♻️ : Dropping ts-jest removes type checking from npm test, and tests/ remains outside tsconfig.json include so vitest/globals types do not reach the test files
    • crates/bitwarden-wasm-internal/integration-tests/tsconfig.json:14

Dependency Changes

Package Change Ecosystem
vitest New (3.2.x) npm
jest Removed npm
ts-jest Removed npm
@types/jest Removed npm

package-lock.json was regenerated alongside the manifest change, as expected.

"types": ["jest", "node"]
"types": ["vitest/globals", "node"]
},
"include": ["*.ts"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ DEBT: Test files are no longer type-checked, and the new vitest/globals types don't reach them.

Details and fix

Two related gaps:

  1. include: ["*.ts"] only matches top-level files, so tests/**/*.test.ts are outside this project. The "types": ["vitest/globals", ...] change therefore does not apply to the test files themselves (editors fall back to an inferred project, where describe/it/expect are untyped).
  2. ts-jest reported TypeScript diagnostics as test failures by default, so npm test used to type-check the suite. Vitest transpiles via esbuild without type checking, so a type error in a test file (e.g. an FFI signature that changed shape) now passes CI silently — which is precisely what this suite exists to catch.
  "include": ["*.ts", "tests/**/*.ts"]

plus a script wired into CI (or into test):

  "scripts": {
    "test": "vitest run",
    "typecheck": "tsc --noEmit"
  }

"ts-jest": "^29.1.0",
"typescript": "^5.6.0"
"typescript": "^5.6.0",
"vitest": "^3.2.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ IMPORTANT: vitest is net-new to this repo and no AppSec approval signal is referenced.

Details

vitest (plus its vite/rollup/esbuild transitive tree) does not appear in any other manifest in this repository, so this is a new dependency rather than a version bump. Bitwarden's process applies to all new dependencies, including dev and test tooling.

The PR description covers the justification and what it replaces, but does not reference a VULN task or AppSec sign-off. Please reach out to AppSec to initiate the Dependency Review and Approval process (or link the existing approval if one was already obtained).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant