test(app): migrate the test app from karma/mocha to vitest-nativescript - #8
Merged
Conversation
- Replace the mocha/karma on-device suite with Vitest driven by @cross-code/vitest-nativescript: a custom pool that launches the app with `ns run ios|android --emulator --no-hmr --env.vitestNativeScript` and runs the specs in a NativeScript Worker over websockets (@valor/nativescript-websockets), with results rendered through @cross-code/vitest-nativescript-ui - Add per-platform configs (vitest.ios/android.config.mts) and the app-side entry points (app/vitest-nativescript.ts + .worker.ts); remove app/test.ts and karma.conf.js - Extend @cross-code/vitest-nativescript: web-event-polyfill for the device runtime, coordinator/worker/expect fixes, webpack config updates, chai -> @vitest/expect - Update the app's project.json targets, CI jobs (Mocha -> Vitest via nx targets), App_Resources manifests, and the workspace docs
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThe pull request migrates the NativeScript WASM test app from Mocha/Karma to Vitest. It adds NativeScript Vitest bootstrapping, worker coordination, coverage support, platform configuration, migrated test suites, workflow updates, documentation changes, and a cleanup script. ChangesVitest NativeScript migration
Workspace tooling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Vitest
participant NativeScriptVitestCoordinator
participant NativeScriptWorker
participant WebSocket
Vitest->>NativeScriptVitestCoordinator: create coordinator and worker
NativeScriptWorker->>NativeScriptVitestCoordinator: send runtime-ready
NativeScriptVitestCoordinator->>WebSocket: wait for socket readiness
NativeScriptVitestCoordinator->>NativeScriptWorker: send start command
NativeScriptWorker->>Vitest: execute specs and report coverage
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
The wasm3-ios and wamr-ios jobs called actions/setup-node@v5 without pnpm/action-setup@v4 first. On macOS runners with corepack disabled in Node 24, setup-node fails with "Unable to locate executable file: pnpm" when cache: 'pnpm' needs to resolve the store path or when any subsequent step invokes pnpm.
Add tools/clean.mjs (zx) that clears all workspace build caches: TypeScript dist/out-tsc, Gradle build/.gradle dirs, Rust cargo targets, Nx cache, NativeScript generated platforms, verdaccio, pnpm-store. Wired as pnpm clean.cache and pnpm clean.cache:all (includes node_modules trees).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
`@apps/nativescript-wasm-test/App_Resources/Android/src/main/AndroidManifest.xml`:
- Line 22: The AndroidManifest.xml site at
apps/nativescript-wasm-test/App_Resources/Android/src/main/AndroidManifest.xml:22
must restrict cleartext traffic to the local Vitest bridge instead of enabling
it for all destinations, using the platform-supported scoped configuration. At
apps/nativescript-wasm-test/App_Resources/iOS/Info.plist:25-31, remove
NSAllowsArbitraryLoads and retain NSAllowsLocalNetworking only if required by
that bridge.
In `@apps/nativescript-wasm-test/app/vitest-nativescript.ts`:
- Around line 6-8: Update the NativeScript Vitest coordinator setup and its
option-resolution path so the WebSocket server defaults to loopback rather than
0.0.0.0. Preserve an explicit physical-device host only when authenticated and
encrypted protection is configured, and ensure WebSocketNativeScriptPoolSession
does not expose unauthenticated control frames.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4aebd911-c22d-4f13-ab7b-03e3375ce5b0
⛔ Files ignored due to path filters (2)
apps/nativescript-wasm-test/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
.github/workflows/ci.yml.github/workflows/code-review.ymlAGENTS.mdREADME.mdapps/nativescript-wasm-test/AGENTS.mdapps/nativescript-wasm-test/App_Resources/Android/src/main/AndroidManifest.xmlapps/nativescript-wasm-test/App_Resources/iOS/Info.plistapps/nativescript-wasm-test/README.mdapps/nativescript-wasm-test/app/main-view-model.tsapps/nativescript-wasm-test/app/test.tsapps/nativescript-wasm-test/app/tests/wamr/fixture-module.spec.tsapps/nativescript-wasm-test/app/tests/wamr/globals-module.spec.tsapps/nativescript-wasm-test/app/tests/wasm3/fixture-module.spec.tsapps/nativescript-wasm-test/app/tests/wasm3/globals-module.spec.tsapps/nativescript-wasm-test/app/vitest-nativescript.tsapps/nativescript-wasm-test/app/vitest-nativescript.worker.tsapps/nativescript-wasm-test/app/wasm/fixture-suite.tsapps/nativescript-wasm-test/karma.conf.jsapps/nativescript-wasm-test/package.jsonapps/nativescript-wasm-test/pnpm-workspace.yamlapps/nativescript-wasm-test/project.jsonapps/nativescript-wasm-test/tsconfig.jsonapps/nativescript-wasm-test/tsconfig.spec.jsonapps/nativescript-wasm-test/vitest.android.config.mtsapps/nativescript-wasm-test/vitest.ios.config.mtsapps/nativescript-wasm-test/webpack.config.jsopencode.jsonpackages/vitest-nativescript-ui/package.jsonpackages/vitest-nativescript-ui/src/lib/results-view.tspackages/vitest-nativescript/README.mdpackages/vitest-nativescript/package.jsonpackages/vitest-nativescript/src/runtime/coordinator.spec.tspackages/vitest-nativescript/src/runtime/coordinator.tspackages/vitest-nativescript/src/runtime/expect.tspackages/vitest-nativescript/src/runtime/web-event-polyfill.spec.tspackages/vitest-nativescript/src/runtime/web-event-polyfill.tspackages/vitest-nativescript/src/runtime/worker.spec.tspackages/vitest-nativescript/src/runtime/worker.tspackages/vitest-nativescript/webpack.cjs
💤 Files with no reviewable changes (3)
- packages/vitest-nativescript/package.json
- apps/nativescript-wasm-test/karma.conf.js
- apps/nativescript-wasm-test/app/test.ts
| android:icon="@mipmap/ic_launcher" | ||
| android:label="@string/app_name" | ||
| android:theme="@style/AppTheme" | ||
| android:usesCleartextTraffic="true" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Identify plaintext endpoints and the platform policy that permits them.
rg -n -C 3 'https?://|wss?://|usesCleartextTraffic|NSAllowsArbitraryLoads|NSAllowsLocalNetworking' \
apps/nativescript-wasm-test \
packages/vitest-nativescript
# Confirm that both test configurations use the local bridge port.
rg -n -C 2 'port\s*=\s*17_878|vitestNativeScriptPort' \
apps/nativescript-wasm-test/vitest.android.config.mts \
apps/nativescript-wasm-test/vitest.ios.config.mtsRepository: listepo/cross-code
Length of output: 12899
Security Misconfiguration (CWE-319): Cleartext Transmission of Sensitive Information
Restrict cleartext transport to the local Vitest bridge.
android:usesCleartextTraffic="true" permits plaintext traffic to any destination. Scope the Android exception to the bridge, and remove NSAllowsArbitraryLoads from apps/nativescript-wasm-test/App_Resources/iOS/Info.plist. Retain NSAllowsLocalNetworking only when required by the bridge.
📍 Affects 2 files
apps/nativescript-wasm-test/App_Resources/Android/src/main/AndroidManifest.xml#L22-L22(this comment)apps/nativescript-wasm-test/App_Resources/iOS/Info.plist#L25-L31
🤖 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
`@apps/nativescript-wasm-test/App_Resources/Android/src/main/AndroidManifest.xml`
at line 22, The AndroidManifest.xml site at
apps/nativescript-wasm-test/App_Resources/Android/src/main/AndroidManifest.xml:22
must restrict cleartext traffic to the local Vitest bridge instead of enabling
it for all destinations, using the platform-supported scoped configuration. At
apps/nativescript-wasm-test/App_Resources/iOS/Info.plist:25-31, remove
NSAllowsArbitraryLoads and retain NSAllowsLocalNetworking only if required by
that bridge.
Comment on lines
+6
to
+8
| const coordinator = new NativeScriptVitestCoordinator({ | ||
| createWorker: () => new Worker('./vitest-nativescript.worker.ts'), | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline packages/vitest-nativescript/src/runtime/coordinator.ts --items all
sed -n '1,260p' packages/vitest-nativescript/src/runtime/coordinator.ts
rg -n -C 3 \
'defaultNativeScriptVitestUrl|NativeScriptVitestCoordinator|vitestNativeScript|17878|usesCleartextTraffic|NSAppTransportSecurity' \
apps packagesRepository: listepo/cross-code
Length of output: 27983
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- WebSocket server and endpoint implementation ---'
rg -n -C 5 \
'WebSocketServer|createServer|listen\(|DEFAULT_NATIVE_SCRIPT_VITEST_PORT|17878|server\.on|handle.*socket|NativeScriptVitestWireMessage|configure' \
packages apps \
-g '*.ts' -g '*.mts' -g '*.cts' -g '*.js' -g '*.cjs' -g '*.md'
printf '%s\n' '--- Relevant file list ---'
git ls-files | rg 'vitest-nativescript|nativescript-wasm-test' | rg '(server|pool|protocol|coordinator|webpack|README|AGENTS|package|manifest|Info\.plist)'Repository: listepo/cross-code
Length of output: 44498
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Session server and connection handling ---'
sed -n '1,245p' packages/vitest-nativescript/src/node/session.ts
printf '%s\n' '--- Resolved host configuration ---'
sed -n '1,125p' packages/vitest-nativescript/src/node/options.ts
printf '%s\n' '--- Host-related tests and documentation ---'
rg -n -C 5 'host|url|physical device|192\.168|WebSocketNativeScriptPoolSession|acceptConnection' \
packages/vitest-nativescript apps/nativescript-wasm-test \
-g '*.ts' -g '*.mts' -g '*.md'Repository: listepo/cross-code
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
options = Path("packages/vitest-nativescript/src/node/options.ts").read_text()
session = Path("packages/vitest-nativescript/src/node/session.ts").read_text()
coordinator = Path("packages/vitest-nativescript/src/runtime/coordinator.ts").read_text()
assert "host: options.host ?? '0.0.0.0'" in options
assert "host: this.options.host" in session
assert "new WebSocketServer({" in session
assert "ws://${isAndroidRuntime ? '10.0.2.2' : '127.0.0.1}:${port}" in coordinator
assert "case 'hello':" in session
assert "kind: 'configure'" in session
assert "case 'worker-message':" in session
assert "if (message.kind === 'worker-message')" in coordinator
print("default server host: 0.0.0.0")
print("server forwards configured host to WebSocketServer")
print("coordinator defaults: iOS 127.0.0.1, Android emulator 10.0.2.2")
print("protocol has no authentication or encryption field in the inspected control flow")
print("unauthenticated network access can reach worker control messages")
PYRepository: listepo/cross-code
Length of output: 245
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
checks = {
"default host": (
"host: options.host ?? '0.0.0.0'",
Path("packages/vitest-nativescript/src/node/options.ts").read_text(),
),
"server host forwarding": (
"host: this.options.host",
Path("packages/vitest-nativescript/src/node/session.ts").read_text(),
),
"WebSocketServer construction": (
"new WebSocketServer({",
Path("packages/vitest-nativescript/src/node/session.ts").read_text(),
),
"coordinator default URL": (
"10.0.2.2",
Path("packages/vitest-nativescript/src/runtime/coordinator.ts").read_text(),
),
"hello handling": (
"case 'hello':",
Path("packages/vitest-nativescript/src/node/session.ts").read_text(),
),
"configure frame": (
"kind: 'configure'",
Path("packages/vitest-nativescript/src/node/session.ts").read_text(),
),
"session worker frame handling": (
"case 'worker-message':",
Path("packages/vitest-nativescript/src/node/session.ts").read_text(),
),
"coordinator worker frame handling": (
"message.kind === 'worker-message'",
Path("packages/vitest-nativescript/src/runtime/coordinator.ts").read_text(),
),
}
for name, (needle, text) in checks.items():
print(f"{name}: {'PASS' if needle in text else 'FAIL'}")
if needle not in text:
for line_no, line in enumerate(text.splitlines(), 1):
if "worker-message" in line or "ws://" in line or "host:" in line:
print(f" {line_no}: {line}")
PYRepository: listepo/cross-code
Length of output: 388
Authorization Bypass (CWE-284)
Reachability: External · Exploitability: Moderate
Bind the WebSocket server to loopback by default
resolveNativeScriptUnitPluginOptions() defaults host to 0.0.0.0, and WebSocketNativeScriptPoolSession passes it directly to WebSocketServer without authentication or encryption. A network peer can send configure or worker-message frames to control the test runner. Default to loopback and require authenticated protection for physical-device connections.
🤖 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 `@apps/nativescript-wasm-test/app/vitest-nativescript.ts` around lines 6 - 8,
Update the NativeScript Vitest coordinator setup and its option-resolution path
so the WebSocket server defaults to loopback rather than 0.0.0.0. Preserve an
explicit physical-device host only when authenticated and encrypted protection
is configured, and ensure WebSocketNativeScriptPoolSession does not expose
unauthenticated control frames.
- Move the fileURLToPath import to the top of the file (healed by ESM hoisting but bad practice at the bottom) - Add missing build-cache paths: packages/*/node_modules (--all) and apps/nativescript-wasm-test/hooks (generated CLI hooks) - Report actual remove/fail counts instead of pattern count
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@tools/clean.mjs`:
- Around line 69-71: Update the total calculation in the clean script to use
dirs.length directly, since the --all flow already appends nodeModules to dirs.
Remove the conditional nodeModules.length addition while preserving the existing
completion message.
- Around line 59-64: Update the cleanup loop around fs.rm to record whether any
deletion failed, set process.exitCode to 1 after the loop when a failure
occurred, and preserve the existing FAIL output in the catch block.
- Around line 40-45: Update the nodeModules list in the clean script to include
the package-local packages/*/node_modules pattern, including
packages/nativescript-wasm3/node_modules and
packages/nativescript-wamr/node_modules, so the --all cleanup removes those
directories.
- Around line 50-60: Update the cleanup loop around glob and fs.rm to prevent
symlink escapes: disable symlink following in the glob options, resolve the
candidate with its real path, and verify that it remains contained within ROOT
before removal. Skip any match whose resolved path is outside ROOT, while
preserving the existing dryRun and recursive deletion behavior for safe matches.
- Around line 50-53: Update the glob call in the cleanup loop over dirs to use
ROOT as cwd, disable directory expansion, and allow directory matches by setting
cwd, expandDirectories: false, and onlyFiles: false. Keep the existing
path.join(ROOT, match) deletion flow unchanged.
- Around line 18-21: Add cleanup patterns in the clean configuration near the
existing packages/*/out-tsc entry to cover all ignored generated tmp paths,
including the generated root and app-specific paths. Ensure clean.cache removes
both out-tsc and tmp outputs consistently, unless an explicit documented reason
requires excluding them.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 75aa9208-5045-4fb9-aad0-50b268b6f6b0
📒 Files selected for processing (3)
.github/workflows/ci.ymlpackage.jsontools/clean.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
…K to 21 - Build both nativescript-wasm3 and nativescript-wamr (not just wasm3) in the wasm-test-android job — the vitest migration links both plugins - Use JDK 21 (temurin) consistently with the other Android jobs and .tool-versions (was using 17) - Update stale comment claiming WAMR tests will fail — both plugins now ship committed .aar files and wamr has the nativescript field
vitest-nativescript / test app: - Add Istanbul coverage via babel-plugin-istanbul + babel-loader in the vitest-nativescript pool; wire @vitest/coverage-istanbul in the app - Expose test.ios/android:coverage scripts in native scripting wasm-test - Extend webpack config, worker, options, and plugin for coverage instrumentation; update vitest configs with coverage reporters tools/clean.mjs: - Add missing cache paths: root out-tsc/tmp, app out-tsc/tmp/test-output - Harden glob: resolve from ROOT, expandDirectories: false, onlyFiles: false, followSymbolicLinks: false - Prevent symlink escapes: resolve realpath and skip matches outside ROOT - Signal failure with process.exitCode = 1 when any deletion fails Add apps/nativescript-wasm-test/test-output/ to root .gitignore.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ns run ios|android --emulator --no-hmr --env.vitestNativeScriptand runs the specs in a NativeScript Worker over websockets (@valor/nativescript-websockets), with results rendered through @cross-code/vitest-nativescript-uiSummary by CodeRabbit
New Features
Bug Fixes
Documentation