feat(e2e): cross-platform E2E — macOS (Appium), Windows (tauri-driver), app-lifecycle spec#418
Closed
CodeGhost21 wants to merge 5 commits intotinyhumansai:mainfrom
Closed
feat(e2e): cross-platform E2E — macOS (Appium), Windows (tauri-driver), app-lifecycle spec#418CodeGhost21 wants to merge 5 commits intotinyhumansai:mainfrom
CodeGhost21 wants to merge 5 commits intotinyhumansai:mainfrom
Conversation
…iver), and app-lifecycle spec - Enable macOS E2E CI job (Appium Mac2) — was fully commented out; add sidecar staging step, Cargo cache, and spec runner matching Linux pattern - Add Windows E2E CI job (tauri-driver on windows-latest, Git Bash shell) - Add app/test/e2e/specs/app-lifecycle.spec.ts covering download artifact checks, installation/launch verification, and update/reinstall scenarios across macOS, Linux, and Windows - Fix e2e-build.sh: set GGML_NATIVE=OFF to fix whisper-rs-sys -mcpu=native build failure under Apple Clang 16 on arm64 - Fix e2e-run-spec.sh: add Windows OS detection (MINGW/MSYS), platform-specific process kill, app data cleanup, config dir, and tauri-driver selection - Fix e2e-resolve-node-appium.sh: prefer Appium co-located with Node 24 and invoke it via explicit NODE24 binary to avoid shebang picking up wrong version - Fix e2e-build.sh: add Windows case (debug binary, no bundle) alongside existing Linux and macOS paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds active macOS and Windows E2E CI jobs, expands cross-platform E2E scripts and driver resolution, and introduces extensive multi-platform WDIO E2E tests and helper changes to handle platform-specific flows and runtime guards. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as Runner
participant Toolchain as Node24+Rust
participant Cache as Cache/Artifact Store
participant Builder as e2e-build / Tauri
participant Sidecar as Rust Sidecar
participant Driver as Appium / tauri-driver
participant Test as WDIO Specs
GH->>Runner: trigger e2e-macos / e2e-windows job
Runner->>Toolchain: install Node 24 & Rust 1.93
Runner->>Cache: compute Cargo.lock hash, restore cache
Runner->>Builder: run e2e-build (platform-specific)
Builder->>Sidecar: build and stage sidecar into bundle/bin
Runner->>Driver: install/start Appium or tauri-driver
Driver->>Test: driver ready
Test->>Sidecar: launch app and execute per-spec flows
Test-->>Runner: report per-spec failures
Runner->>Cache: upload logs/artifacts on failure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Both e2e-macos and e2e-windows were only triggered on push to main or manual workflow_dispatch, causing them to be skipped on PRs. Add pull_request as a trigger condition for both jobs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
app/test/e2e/specs/app-lifecycle.spec.ts (1)
632-650: UseclickNativeButton()for this onboarding step.The raw DOM query/click path hard-codes a tauri-driver-only interaction and bypasses the shared E2E button helper. Reusing the helper keeps the step aligned with the rest of the cross-platform suite.
As per coding guidelines "Use `clickNativeButton()`, `hasAppChrome()`, `waitForWebView()`, `clickToggle()` helper functions for cross-platform element interaction in E2E tests".Suggested fix
import { + clickNativeButton, dumpAccessibilityTree, hasAppChrome, textExists, waitForWebView, waitForWindowVisible, } from '../helpers/element-helpers'; @@ - await browser.execute(() => { - const buttons = Array.from(document.querySelectorAll('button')); - const skip = buttons.find(b => b.textContent?.includes('Continue Without Permission')); - skip?.click(); - }); + await clickNativeButton('Continue Without Permission'); await browser.pause(2_000);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/test/e2e/specs/app-lifecycle.spec.ts` around lines 632 - 650, The test currently uses a raw DOM query via browser.execute to click the "Continue Without Permission" button (inside app-lifecycle.spec.ts) which bypasses the shared E2E helper; replace that browser.execute block with the cross-platform helper clickNativeButton('Continue Without Permission') (keeping the preceding isTauriDriver() guard and the textExists check), ensure you await clickNativeButton and remove the direct DOM query/skip variable, and use the existing helper functions (clickNativeButton, textExists) so the onboarding step uses the shared cross-platform interaction path..github/workflows/test.yml (2)
321-327: Separate the Windows dispatch gate from the macOS flag.This job currently keys off
run_macos_e2e, so a manual Windows-only validation also schedules the macOS runner. A dedicated Windows input would make the opt-in behavior match the job comments and the runner-cost gating you added.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 321 - 327, The e2e-windows job is gated by the macOS input flag (run_macos_e2e); add a separate workflow input (e.g., run_windows_e2e) and update the e2e-windows job's if condition to use that new input instead of run_macos_e2e so manual dispatches can opt into Windows-only runs; specifically add the new input in the workflow inputs block and change the e2e-windows job (symbol: e2e-windows) conditional expression to reference run_windows_e2e for workflow_dispatch while keeping the push-to-main branch check intact.
248-257: Cacheapp/src-tauri/targettoo.Both new jobs later consume artifacts from
app/src-tauri/target/debug, but the cache only persists the repo-roottargetdirectory. That means the heavy Tauri build outputs still miss the cache and get rebuilt from scratch.Suggested fix
- name: Cache Cargo registry and build uses: actions/cache@v4 with: path: | ~/.cargo/registry ~/.cargo/git target + app/src-tauri/targetAlso applies to: 356-365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 248 - 257, The workflow cache step named "Cache Cargo registry and build" is missing the Tauri build outputs; update the cache `with.path` to include "app/src-tauri/target" in addition to "target", "~/.cargo/registry", and "~/.cargo/git" so artifacts from app/src-tauri/target/debug are persisted, and apply the same change to the other similar cargo cache block (the second "Cache Cargo registry and build" usage) to ensure both jobs reuse the Tauri build cache.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/scripts/e2e-build.sh`:
- Around line 21-24: The script exports GGML_NATIVE=OFF before sourcing the
project's dotenv file, but that .env is sourced later and can override
GGML_NATIVE; move or repeat the export so GGML_NATIVE=OFF is applied after the
dotenv is loaded (or re-export GGML_NATIVE=OFF immediately after the dotenv
sourcing call) to ensure Apple Clang arm64 CPU-detection is disabled; update the
e2e-build.sh logic around the dotenv source and the GGML_NATIVE export
(reference GGML_NATIVE and the dotenv/.env sourcing) to enforce the final value.
In `@app/test/e2e/specs/app-lifecycle.spec.ts`:
- Around line 740-753: The test is currently checking the mock server health on
a hard-coded port instead of validating the updater payload; update the test
(the spec named "mock update server returns a valid latest.json response" that
calls setMockBehavior('update','available') and resetMockBehavior()) to fetch
the actual mocked update descriptor URL (use the E2E_MOCK_PORT or the test
harness' mock port config rather than 18473) — request the
/releases/latest/download/latest.json path, parse the JSON, and assert required
fields (e.g., version, url, releaseNotes or whatever the updater expects) have
correct types and values; if the fetch or shape check fails, capture diagnostics
(response text, request logs, and optionally dumpAccessibilityTree()) before
failing to aid debugging.
- Around line 232-240: The Windows probe currently skips the version assertion
when PowerShell fails (the if (success) guard), causing false positives; modify
the test to fail when safeExec indicates failure (e.g., assert or throw if
success is falsy) before parsing stdout, then proceed to split stdout and assert
major >= 10; reference the safeExec call and the variables stdout and success
and replace the conditional guard with an explicit failure on !success followed
by the existing version parsing/assertion.
- Around line 281-318: The test only snapshots priorAuth but also overwrites
persist:user and unconditionally removes it in the finally block, mutating
global state for later tests; update the setup to also snapshot the original
persist:user (e.g., const priorUser = await browser.execute(() =>
localStorage.getItem('persist:user') ?? null)), write the corrupted persist:user
as now, and in the finally callback restore both keys using the saved priorAuth
and priorUser values (set the item if saved !== null, otherwise removeItem),
ensuring both persist:auth and persist:user are returned to their original
values after the test.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 321-327: The e2e-windows job is gated by the macOS input flag
(run_macos_e2e); add a separate workflow input (e.g., run_windows_e2e) and
update the e2e-windows job's if condition to use that new input instead of
run_macos_e2e so manual dispatches can opt into Windows-only runs; specifically
add the new input in the workflow inputs block and change the e2e-windows job
(symbol: e2e-windows) conditional expression to reference run_windows_e2e for
workflow_dispatch while keeping the push-to-main branch check intact.
- Around line 248-257: The workflow cache step named "Cache Cargo registry and
build" is missing the Tauri build outputs; update the cache `with.path` to
include "app/src-tauri/target" in addition to "target", "~/.cargo/registry", and
"~/.cargo/git" so artifacts from app/src-tauri/target/debug are persisted, and
apply the same change to the other similar cargo cache block (the second "Cache
Cargo registry and build" usage) to ensure both jobs reuse the Tauri build
cache.
In `@app/test/e2e/specs/app-lifecycle.spec.ts`:
- Around line 632-650: The test currently uses a raw DOM query via
browser.execute to click the "Continue Without Permission" button (inside
app-lifecycle.spec.ts) which bypasses the shared E2E helper; replace that
browser.execute block with the cross-platform helper clickNativeButton('Continue
Without Permission') (keeping the preceding isTauriDriver() guard and the
textExists check), ensure you await clickNativeButton and remove the direct DOM
query/skip variable, and use the existing helper functions (clickNativeButton,
textExists) so the onboarding step uses the shared cross-platform interaction
path.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0ecdfeda-07fa-49f7-90a8-a2e70e272cbb
📒 Files selected for processing (5)
.github/workflows/test.ymlapp/scripts/e2e-build.shapp/scripts/e2e-resolve-node-appium.shapp/scripts/e2e-run-spec.shapp/test/e2e/specs/app-lifecycle.spec.ts
Comment on lines
+21
to
+24
| # Disable ggml/whisper.cpp native CPU detection (-mcpu=native) which is | ||
| # unsupported by Apple Clang 16 on arm64 and unnecessary for E2E test builds. | ||
| # whisper-rs-sys build.rs forwards any GGML_* env var as a CMake define. | ||
| export GGML_NATIVE=OFF |
Contributor
There was a problem hiding this comment.
Re-apply GGML_NATIVE=OFF after dotenv loading.
Because .env is sourced later at Line 37, a local GGML_NATIVE=... entry can still overwrite this export and bring back the Apple Clang arm64 failure this change is trying to prevent.
Suggested fix
# whisper-rs-sys build.rs forwards any GGML_* env var as a CMake define.
-export GGML_NATIVE=OFF
echo "Building E2E app with VITE_BACKEND_URL=$VITE_BACKEND_URL"
@@
if [ -f .env ]; then
# shellcheck source=/dev/null
source "$REPO_ROOT/scripts/load-dotenv.sh"
else
echo "No .env file — skipping load-dotenv (optional for CI)."
fi
+export GGML_NATIVE=OFF
export VITE_BACKEND_URL="http://127.0.0.1:${E2E_MOCK_PORT:-18473}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/scripts/e2e-build.sh` around lines 21 - 24, The script exports
GGML_NATIVE=OFF before sourcing the project's dotenv file, but that .env is
sourced later and can override GGML_NATIVE; move or repeat the export so
GGML_NATIVE=OFF is applied after the dotenv is loaded (or re-export
GGML_NATIVE=OFF immediately after the dotenv sourcing call) to ensure Apple
Clang arm64 CPU-detection is disabled; update the e2e-build.sh logic around the
dotenv source and the GGML_NATIVE export (reference GGML_NATIVE and the
dotenv/.env sourcing) to enforce the final value.
Comment on lines
+232
to
+240
| const { stdout, success } = safeExec( | ||
| 'powershell -Command "[System.Environment]::OSVersion.Version.ToString()"' | ||
| ); | ||
| console.log(`[AppLifecycle][0.1.2] Windows version: ${stdout}`); | ||
| if (success) { | ||
| const parts = stdout.split('.').map(Number); | ||
| const [major = 0] = parts; | ||
| expect(major).toBeGreaterThanOrEqual(10); | ||
| } |
Contributor
There was a problem hiding this comment.
Fail the Windows requirement check when the PowerShell probe fails.
Right now a broken command still passes this test because the assertion only runs inside if (success). That turns the Windows compatibility check into a false positive on the one platform it is supposed to validate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/test/e2e/specs/app-lifecycle.spec.ts` around lines 232 - 240, The Windows
probe currently skips the version assertion when PowerShell fails (the if
(success) guard), causing false positives; modify the test to fail when safeExec
indicates failure (e.g., assert or throw if success is falsy) before parsing
stdout, then proceed to split stdout and assert major >= 10; reference the
safeExec call and the variables stdout and success and replace the conditional
guard with an explicit failure on !success followed by the existing version
parsing/assertion.
Comment on lines
+281
to
+318
| // Snapshot current auth state so we can restore it after the test | ||
| const priorAuth = await browser.execute(() => localStorage.getItem('persist:auth') ?? null); | ||
|
|
||
| try { | ||
| // Write intentionally malformed data simulating a corrupted installer payload | ||
| await browser.execute(() => { | ||
| localStorage.setItem('persist:auth', 'THIS IS NOT JSON {{{{CORRUPTED}}}}'); | ||
| localStorage.setItem('persist:user', '<<CORRUPTED>>'); | ||
| }); | ||
| console.log('[AppLifecycle][0.1.3] Corrupted state written to localStorage'); | ||
|
|
||
| // Reload the app — redux-persist must handle the parse error gracefully | ||
| await browser.execute(() => window.location.reload()); | ||
| await browser.pause(3_000); | ||
| await waitForAppReady(20_000); | ||
|
|
||
| // App must still render (not a blank screen or error boundary crash) | ||
| const appAlive = await hasAppChrome(); | ||
| expect(appAlive).toBe(true); | ||
|
|
||
| // Verify corrupted auth was cleared / reset rather than retained | ||
| const authAfter = await browser.execute(() => localStorage.getItem('persist:auth') ?? null); | ||
| console.log( | ||
| '[AppLifecycle][0.1.3] Auth after reload:', | ||
| authAfter ? authAfter.slice(0, 100) : '(null)' | ||
| ); | ||
| // Must not still hold the raw corrupted string | ||
| expect(authAfter).not.toBe('THIS IS NOT JSON {{{{CORRUPTED}}}}'); | ||
| } finally { | ||
| // Restore the prior auth state regardless of test outcome | ||
| await browser.execute((saved: string | null) => { | ||
| if (saved !== null) { | ||
| localStorage.setItem('persist:auth', saved); | ||
| } else { | ||
| localStorage.removeItem('persist:auth'); | ||
| } | ||
| localStorage.removeItem('persist:user'); | ||
| }, priorAuth); |
Contributor
There was a problem hiding this comment.
Restore persist:user too.
This case snapshots persist:auth, but it also overwrites persist:user and then always deletes it in finally. If the session started with persisted user state, later tests in this spec now run against a mutated store.
Suggested fix
- const priorAuth = await browser.execute(() => localStorage.getItem('persist:auth') ?? null);
+ const [priorAuth, priorUser] = await browser.execute(() => [
+ localStorage.getItem('persist:auth') ?? null,
+ localStorage.getItem('persist:user') ?? null,
+ ]);
@@
- await browser.execute((saved: string | null) => {
- if (saved !== null) {
- localStorage.setItem('persist:auth', saved);
+ await browser.execute((savedAuth: string | null, savedUser: string | null) => {
+ if (savedAuth !== null) {
+ localStorage.setItem('persist:auth', savedAuth);
} else {
localStorage.removeItem('persist:auth');
}
- localStorage.removeItem('persist:user');
- }, priorAuth);
+ if (savedUser !== null) {
+ localStorage.setItem('persist:user', savedUser);
+ } else {
+ localStorage.removeItem('persist:user');
+ }
+ }, priorAuth, priorUser);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Snapshot current auth state so we can restore it after the test | |
| const priorAuth = await browser.execute(() => localStorage.getItem('persist:auth') ?? null); | |
| try { | |
| // Write intentionally malformed data simulating a corrupted installer payload | |
| await browser.execute(() => { | |
| localStorage.setItem('persist:auth', 'THIS IS NOT JSON {{{{CORRUPTED}}}}'); | |
| localStorage.setItem('persist:user', '<<CORRUPTED>>'); | |
| }); | |
| console.log('[AppLifecycle][0.1.3] Corrupted state written to localStorage'); | |
| // Reload the app — redux-persist must handle the parse error gracefully | |
| await browser.execute(() => window.location.reload()); | |
| await browser.pause(3_000); | |
| await waitForAppReady(20_000); | |
| // App must still render (not a blank screen or error boundary crash) | |
| const appAlive = await hasAppChrome(); | |
| expect(appAlive).toBe(true); | |
| // Verify corrupted auth was cleared / reset rather than retained | |
| const authAfter = await browser.execute(() => localStorage.getItem('persist:auth') ?? null); | |
| console.log( | |
| '[AppLifecycle][0.1.3] Auth after reload:', | |
| authAfter ? authAfter.slice(0, 100) : '(null)' | |
| ); | |
| // Must not still hold the raw corrupted string | |
| expect(authAfter).not.toBe('THIS IS NOT JSON {{{{CORRUPTED}}}}'); | |
| } finally { | |
| // Restore the prior auth state regardless of test outcome | |
| await browser.execute((saved: string | null) => { | |
| if (saved !== null) { | |
| localStorage.setItem('persist:auth', saved); | |
| } else { | |
| localStorage.removeItem('persist:auth'); | |
| } | |
| localStorage.removeItem('persist:user'); | |
| }, priorAuth); | |
| // Snapshot current auth state so we can restore it after the test | |
| const [priorAuth, priorUser] = await browser.execute(() => [ | |
| localStorage.getItem('persist:auth') ?? null, | |
| localStorage.getItem('persist:user') ?? null, | |
| ]); | |
| try { | |
| // Write intentionally malformed data simulating a corrupted installer payload | |
| await browser.execute(() => { | |
| localStorage.setItem('persist:auth', 'THIS IS NOT JSON {{{{CORRUPTED}}}}'); | |
| localStorage.setItem('persist:user', '<<CORRUPTED>>'); | |
| }); | |
| console.log('[AppLifecycle][0.1.3] Corrupted state written to localStorage'); | |
| // Reload the app — redux-persist must handle the parse error gracefully | |
| await browser.execute(() => window.location.reload()); | |
| await browser.pause(3_000); | |
| await waitForAppReady(20_000); | |
| // App must still render (not a blank screen or error boundary crash) | |
| const appAlive = await hasAppChrome(); | |
| expect(appAlive).toBe(true); | |
| // Verify corrupted auth was cleared / reset rather than retained | |
| const authAfter = await browser.execute(() => localStorage.getItem('persist:auth') ?? null); | |
| console.log( | |
| '[AppLifecycle][0.1.3] Auth after reload:', | |
| authAfter ? authAfter.slice(0, 100) : '(null)' | |
| ); | |
| // Must not still hold the raw corrupted string | |
| expect(authAfter).not.toBe('THIS IS NOT JSON {{{{CORRUPTED}}}}'); | |
| } finally { | |
| // Restore the prior auth state regardless of test outcome | |
| await browser.execute((savedAuth: string | null, savedUser: string | null) => { | |
| if (savedAuth !== null) { | |
| localStorage.setItem('persist:auth', savedAuth); | |
| } else { | |
| localStorage.removeItem('persist:auth'); | |
| } | |
| if (savedUser !== null) { | |
| localStorage.setItem('persist:user', savedUser); | |
| } else { | |
| localStorage.removeItem('persist:user'); | |
| } | |
| }, priorAuth, priorUser); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/test/e2e/specs/app-lifecycle.spec.ts` around lines 281 - 318, The test
only snapshots priorAuth but also overwrites persist:user and unconditionally
removes it in the finally block, mutating global state for later tests; update
the setup to also snapshot the original persist:user (e.g., const priorUser =
await browser.execute(() => localStorage.getItem('persist:user') ?? null)),
write the corrupted persist:user as now, and in the finally callback restore
both keys using the saved priorAuth and priorUser values (set the item if saved
!== null, otherwise removeItem), ensuring both persist:auth and persist:user are
returned to their original values after the test.
Comment on lines
+740
to
+753
| it('mock update server returns a valid latest.json response', async () => { | ||
| // Simulate the GitHub releases latest.json format that Tauri's updater expects. | ||
| // The mock server at /releases/latest/download/latest.json should return | ||
| // a valid update descriptor so the updater can parse it. | ||
| setMockBehavior('update', 'available'); | ||
|
|
||
| // The app's updater is active:false so it won't call this automatically. | ||
| // We verify the mock server itself can serve a properly shaped response. | ||
| const resp = await fetch('http://127.0.0.1:18473/__admin/health').catch(() => null); | ||
| console.log(`[AppLifecycle][0.3.1] Mock server health: ${resp?.status ?? 'unreachable'}`); | ||
| // Mock server is running if this test suite reached here | ||
| expect(resp).not.toBeNull(); | ||
|
|
||
| resetMockBehavior(); |
Contributor
There was a problem hiding this comment.
Validate the mocked updater payload, not /__admin/health.
This test currently pings the admin health endpoint on a hard-coded 18473 port, so it can pass while latest.json is broken and it will fail as soon as E2E_MOCK_PORT is overridden. Hitting the actual mocked update descriptor and asserting its shape would make this a real updater check.
Suggested fix
setMockBehavior('update', 'available');
- // The app's updater is active:false so it won't call this automatically.
- // We verify the mock server itself can serve a properly shaped response.
- const resp = await fetch('http://127.0.0.1:18473/__admin/health').catch(() => null);
- console.log(`[AppLifecycle][0.3.1] Mock server health: ${resp?.status ?? 'unreachable'}`);
- // Mock server is running if this test suite reached here
- expect(resp).not.toBeNull();
+ const mockPort = process.env.E2E_MOCK_PORT ?? '18473';
+ const resp = await fetch(
+ `http://127.0.0.1:${mockPort}/releases/latest/download/latest.json`
+ ).catch(() => null);
+ expect(resp?.ok).toBe(true);
+
+ const payload = await resp!.json();
+ console.log('[AppLifecycle][0.3.1] latest.json payload:', JSON.stringify(payload));
+ expect(payload).toHaveProperty('version');
+ expect(payload).toHaveProperty('platforms');
resetMockBehavior();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('mock update server returns a valid latest.json response', async () => { | |
| // Simulate the GitHub releases latest.json format that Tauri's updater expects. | |
| // The mock server at /releases/latest/download/latest.json should return | |
| // a valid update descriptor so the updater can parse it. | |
| setMockBehavior('update', 'available'); | |
| // The app's updater is active:false so it won't call this automatically. | |
| // We verify the mock server itself can serve a properly shaped response. | |
| const resp = await fetch('http://127.0.0.1:18473/__admin/health').catch(() => null); | |
| console.log(`[AppLifecycle][0.3.1] Mock server health: ${resp?.status ?? 'unreachable'}`); | |
| // Mock server is running if this test suite reached here | |
| expect(resp).not.toBeNull(); | |
| resetMockBehavior(); | |
| it('mock update server returns a valid latest.json response', async () => { | |
| // Simulate the GitHub releases latest.json format that Tauri's updater expects. | |
| // The mock server at /releases/latest/download/latest.json should return | |
| // a valid update descriptor so the updater can parse it. | |
| setMockBehavior('update', 'available'); | |
| const mockPort = process.env.E2E_MOCK_PORT ?? '18473'; | |
| const resp = await fetch( | |
| `http://127.0.0.1:${mockPort}/releases/latest/download/latest.json` | |
| ).catch(() => null); | |
| expect(resp?.ok).toBe(true); | |
| const payload = await resp!.json(); | |
| console.log('[AppLifecycle][0.3.1] latest.json payload:', JSON.stringify(payload)); | |
| expect(payload).toHaveProperty('version'); | |
| expect(payload).toHaveProperty('platforms'); | |
| resetMockBehavior(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/test/e2e/specs/app-lifecycle.spec.ts` around lines 740 - 753, The test is
currently checking the mock server health on a hard-coded port instead of
validating the updater payload; update the test (the spec named "mock update
server returns a valid latest.json response" that calls
setMockBehavior('update','available') and resetMockBehavior()) to fetch the
actual mocked update descriptor URL (use the E2E_MOCK_PORT or the test harness'
mock port config rather than 18473) — request the
/releases/latest/download/latest.json path, parse the JSON, and assert required
fields (e.g., version, url, releaseNotes or whatever the updater expects) have
correct types and values; if the fetch or shape check fails, capture diagnostics
(response text, request logs, and optionally dumpAccessibilityTree()) before
failing to aid debugging.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
294-298: Clarify: Spec platform coverage inconsistency.
app-lifecycle.spec.tscontains platform-specific skip logic and is designed to run across all platforms. Per the PR description, it's "intended to run on macOS, Linux, and Windows," but the Linux job (lines 196–200) runstelegram-flow.spec.tsinstead.Since
app-lifecycle.spec.tshandles platform differences internally viaprocess.platformchecks, it can safely run on any platform. However, if the intent is to ensure it executes on all three platforms as stated, add it to the Linux spec list. Alternatively, clarify whether this separation is intentional (e.g., staged rollout or platform-specific scheduling).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 294 - 298, The Linux CI job currently runs telegram-flow.spec.ts instead of app-lifecycle.spec.ts, causing the platform-coverage mismatch; update the Linux job's spec list in .github/workflows/test.yml so it includes test/e2e/specs/app-lifecycle.spec.ts (and remove or keep telegram-flow.spec.ts only if intentional), or if the separation was deliberate, add a brief comment in the Linux job noting the intentional omission so reviewers know this is by design.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 321-328: The workflow's e2e-windows job condition incorrectly
references the macOS manual input (run_macos_e2e); update the if condition for
the e2e-windows job (e2e-windows) to either reference a new run_windows_e2e
workflow_dispatch input (add input name run_windows_e2e and use it in the if
expression) or simplify the manual-dispatch clause to just github.event_name ==
'workflow_dispatch' so Windows runs on any manual dispatch—modify the job's if
line accordingly to replace run_macos_e2e with the correct Windows input or the
simplified condition.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 294-298: The Linux CI job currently runs telegram-flow.spec.ts
instead of app-lifecycle.spec.ts, causing the platform-coverage mismatch; update
the Linux job's spec list in .github/workflows/test.yml so it includes
test/e2e/specs/app-lifecycle.spec.ts (and remove or keep telegram-flow.spec.ts
only if intentional), or if the separation was deliberate, add a brief comment
in the Linux job noting the intentional omission so reviewers know this is by
design.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7fe253b5-c3c6-4cab-8127-7de08e0ddc3e
📒 Files selected for processing (1)
.github/workflows/test.yml
Comment on lines
+321
to
+328
| e2e-windows: | ||
| name: E2E (Windows / tauri-driver) | ||
| # Run on: PRs, pushes to main, or manual dispatch with opt-in flag. | ||
| # windows-latest runners are ~2× more expensive than Linux. | ||
| if: > | ||
| github.event_name == 'pull_request' || | ||
| github.event_name == 'push' && github.ref == 'refs/heads/main' || | ||
| github.event_name == 'workflow_dispatch' && github.event.inputs.run_macos_e2e == 'true' |
Contributor
There was a problem hiding this comment.
Incorrect input reference in Windows job condition.
Line 328 checks run_macos_e2e for the Windows job, which appears to be a copy-paste error. This means:
- On PRs and main pushes: Windows E2E runs correctly
- On manual
workflow_dispatch: Windows E2E only runs if the macOS flag is enabled
Either add a run_windows_e2e input and reference it here, or if Windows should always run on manual dispatch, simplify to github.event_name == 'workflow_dispatch'.
🔧 Proposed fix: Add Windows-specific input
Add the input at lines 9-16:
run_macos_e2e:
description: 'Run macOS E2E tests (Appium Mac2)'
required: false
default: 'false'
type: choice
options:
- 'false'
- 'true'
+ run_windows_e2e:
+ description: 'Run Windows E2E tests (tauri-driver)'
+ required: false
+ default: 'false'
+ type: choice
+ options:
+ - 'false'
+ - 'true'Then fix line 328:
if: >
github.event_name == 'pull_request' ||
github.event_name == 'push' && github.ref == 'refs/heads/main' ||
- github.event_name == 'workflow_dispatch' && github.event.inputs.run_macos_e2e == 'true'
+ github.event_name == 'workflow_dispatch' && github.event.inputs.run_windows_e2e == 'true'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/test.yml around lines 321 - 328, The workflow's
e2e-windows job condition incorrectly references the macOS manual input
(run_macos_e2e); update the if condition for the e2e-windows job (e2e-windows)
to either reference a new run_windows_e2e workflow_dispatch input (add input
name run_windows_e2e and use it in the if expression) or simplify the
manual-dispatch clause to just github.event_name == 'workflow_dispatch' so
Windows runs on any manual dispatch—modify the job's if line accordingly to
replace run_macos_e2e with the correct Windows input or the simplified
condition.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/test/e2e/helpers/element-helpers.ts`:
- Around line 28-35: The xpathStringLiteral function is incorrectly
HTML-escaping &, <, > before building an XPath string literal which breaks
contains() matching; remove the replaceAll calls so the function operates on the
raw input text and keep the existing quote-handling logic (the early-return
double-quote and single-quote branches and the parts/concat assembly) unchanged
except using the unescaped text variable; update any variable names if needed
(e.g., replace `escaped` with `text`) and ensure only
quote-escaping/concatenation logic remains so values like "Billing & Usage"
match DOM attribute values correctly.
In `@app/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 343-354: The test currently returns early on Mac2 after verifying
a logout entry (using isMac2(), navigateToSettings(), textExists()), which
falsely marks the full logout behavior as passing; change the test to use
Mocha's skip by converting the test function from an arrow to a function
expression so you can call this.skip() when isMac2() is true (or alternatively
split the lightweight "logout entry exists" smoke check into its own test and
keep the full "user can log out via Settings and returns to Welcome" test
intact), ensuring the early-exit uses this.skip() instead of return; apply the
same pattern to the similar early-return occurrences in
conversations-web-channel-flow.spec.ts, card-payment-flow.spec.ts, and
crypto-payment-flow.spec.ts.
In `@app/test/e2e/specs/login-flow.spec.ts`:
- Around line 439-446: The tauri-driver branch only removes persisted auth from
localStorage but doesn't reset the in-memory auth state, so add a hard reload to
fully reset the app before the bypass-auth phase: after calling
localStorage.removeItem('persist:auth') and setting window.location.hash = '/',
either call window.location.reload() inside the browser.execute callback or call
browser.refresh() immediately after the execute to force the app to reinitialize
and clear any in-memory auth state (refer to isTauriDriver, browser.execute,
localStorage.removeItem, window.location.reload/browser.refresh).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1746bc0d-00f7-4145-81d4-74dd72e16c41
📒 Files selected for processing (9)
app/test/e2e/helpers/element-helpers.tsapp/test/e2e/helpers/shared-flows.tsapp/test/e2e/specs/auth-access-control.spec.tsapp/test/e2e/specs/card-payment-flow.spec.tsapp/test/e2e/specs/conversations-web-channel-flow.spec.tsapp/test/e2e/specs/crypto-payment-flow.spec.tsapp/test/e2e/specs/login-flow.spec.tsapp/test/e2e/specs/service-connectivity-flow.spec.tsapp/test/e2e/specs/skills-registry.spec.ts
💤 Files with no reviewable changes (1)
- app/test/e2e/specs/skills-registry.spec.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/test/e2e/specs/auth-access-control.spec.ts (1)
283-289:⚠️ Potential issue | 🟠 MajorUse Mocha skip instead of early return for Mac2 branch in portal test.
At Line 283, this test still exits via
returnon Mac2, which reports the case as passed while skipping its core assertion. Convert thisitcallback toasync function ()and callthis.skip()in the Mac2 guard.Suggested fix
- it('3.3.3 — manage subscription opens Stripe portal', async () => { + it('3.3.3 — manage subscription opens Stripe portal', async function () { if (isMac2()) { console.log( '[AuthAccess] 3.3.3 — skipping portal action assertion on Mac2; header/payment-method actions are not exposed reliably in WKWebView accessibility' ); - return; + this.skip(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/test/e2e/specs/auth-access-control.spec.ts` around lines 283 - 289, The test "3.3.3 — manage subscription opens Stripe portal" currently bails with an early return when isMac2() is true; change the test callback from an arrow async () => to an async function() so you can call this.skip() inside the Mac2 guard, remove the early return, and invoke this.skip() when isMac2() is true to mark the test as skipped; keep the rest of the test body (assertions about opening the Stripe portal) unchanged.
🧹 Nitpick comments (1)
app/test/e2e/specs/conversations-web-channel-flow.spec.ts (1)
59-65: Correct pattern for conditional test skipping.The conversion from arrow function to
functionform is the right approach for Mocha'sthis.skip()context binding. The skip reason clearly documents the Appium WKWebView limitation withbrowser.execute().Minor: consider using the file's
stepLog()helper instead of rawconsole.logfor consistency with the rest of the test's logging.
,♻️ Optional: use stepLog for consistency
if (isMac2()) { - console.log( - '[ConversationsE2E] Skipping on Mac2: composing and submitting chat input requires WebView execute, which Appium WKWebView does not support' + stepLog( + 'Skipping on Mac2: composing and submitting chat input requires WebView execute, which Appium WKWebView does not support' ); this.skip(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/test/e2e/specs/conversations-web-channel-flow.spec.ts` around lines 59 - 65, The test uses function() so this.skip() is valid; keep the function form in the "sends UI message through agent loop and renders response" it-block and ensure the conditional uses isMac2() to call this.skip() with the same descriptive reason; optionally replace the raw console.log call with the file's stepLog() helper for consistent test logging (refer to the same it block and isMac2()/this.skip()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 283-289: The test "3.3.3 — manage subscription opens Stripe
portal" currently bails with an early return when isMac2() is true; change the
test callback from an arrow async () => to an async function() so you can call
this.skip() inside the Mac2 guard, remove the early return, and invoke
this.skip() when isMac2() is true to mark the test as skipped; keep the rest of
the test body (assertions about opening the Stripe portal) unchanged.
---
Nitpick comments:
In `@app/test/e2e/specs/conversations-web-channel-flow.spec.ts`:
- Around line 59-65: The test uses function() so this.skip() is valid; keep the
function form in the "sends UI message through agent loop and renders response"
it-block and ensure the conditional uses isMac2() to call this.skip() with the
same descriptive reason; optionally replace the raw console.log call with the
file's stepLog() helper for consistent test logging (refer to the same it block
and isMac2()/this.skip()).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b0440d5-4714-41bf-9090-113542b9c479
📒 Files selected for processing (6)
app/test/e2e/helpers/element-helpers.tsapp/test/e2e/specs/auth-access-control.spec.tsapp/test/e2e/specs/card-payment-flow.spec.tsapp/test/e2e/specs/conversations-web-channel-flow.spec.tsapp/test/e2e/specs/crypto-payment-flow.spec.tsapp/test/e2e/specs/login-flow.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- app/test/e2e/helpers/element-helpers.ts
- app/test/e2e/specs/card-payment-flow.spec.ts
- app/test/e2e/specs/crypto-payment-flow.spec.ts
- app/test/e2e/specs/login-flow.spec.ts
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.
Summary
e2e-macos) — was fully commented out; adds Cargo cache, sidecar staging (resolves host triple viarustc -vV), and runs core specs via Appium Mac2e2e-windows) —windows-latestrunner, Git Bash shell,tauri-driveron port 4444, same spec set as Linuxapp/test/e2e/specs/app-lifecycle.spec.ts— 26 tests across 3 sections: download artifact checks (0.1), installation & launch verification (0.2), and update/reinstall scenarios (0.3); runs on macOS, Linux, and Windowse2e-build.sh— setGGML_NATIVE=OFFto fixwhisper-rs-sys-mcpu=nativebuild failure under Apple Clang 16 on arm64; add Windows--debug --no-bundlepathe2e-run-spec.sh— add Windows OS detection (MINGW*/MSYS*), platform-specific process kill, app data cleanup, config dir ($APPDATA), and tauri-driver selection for Windowse2e-resolve-node-appium.sh— prefer Appium co-located with Node 24 and invoke via explicit$NODE24binary so#!/usr/bin/env nodeshebang doesn't resolve the wrong Node versionTest plan
yarn workspace openhuman-app test:e2e:buildpasses (GGML_NATIVE fix verified)bash app/scripts/e2e-run-spec.sh test/e2e/specs/smoke.spec.ts smoke— 3/3 passing via Appium Mac2feat/e2e-cross-platform→run_macos_e2e=truee2e-macosjob onmacos-latest🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Chores