test(e2e): cover runtime picker → login → onboarding → logout#1982
Conversation
Exercises the first-launch funnel end-to-end against the mock server: BootCheckGate picker (local + cloud branches, validation, test-connection), post-OAuth deep-link callback, onboarding walk-through, Home reach, and logout back to Welcome. Runs on the existing Appium chromium-driver + CEF/CDP harness used by e2e/docker-compose.yml on Linux.
- clickByTextDom matched on exact text equality; tile buttons concatenate their title+description in textContent so the strict match never fired. Switch to includes() + smallest-descendant scoring so we click the most specific match (the option tile / form button, not its body ancestor). - Replace hardcoded greeting-string home check with a route + 'welcome gone' fallback so post-onboarding Home detection survives copy churn.
📝 WalkthroughWalkthroughAdds a new E2E spec that drives Welcome → runtime picker (Cloud/Local validation) → OAuth deep-link login → optional onboarding → Home, then logs out; includes browser-executed helpers and mock-server assertions. ChangesRuntime Picker and OAuth E2E Test
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser/App
participant MockServer as Mock Server
participant DeepLink as DeepLink Trigger
participant Onboarding as Onboarding Flow
Browser->>Browser: Show Welcome with OAuth providers & runtime selector
Browser->>Browser: Open BootCheckGate runtime picker
Browser->>Browser: Display Cloud and Local option tiles
Browser->>Browser: Show URL & token inputs for Cloud
Browser->>MockServer: Test Connection -> unreachable/auth-failure polling
DeepLink->>Browser: triggerAuthDeepLinkBypass (OAuth callback)
Browser->>MockServer: GET /auth/me
MockServer-->>Browser: auth response
Browser->>Onboarding: optionally walk onboarding
Onboarding-->>Browser: complete
Browser->>Browser: Navigate to Home
Browser->>Browser: Logout via Settings -> return to Welcome
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/test/e2e/specs/runtime-picker-login.spec.ts (1)
267-278: ⚡ Quick winAdd diagnostics around the provider-button probe.
If this assertion fails in CI, it currently produces a bare boolean failure with no tree or button-label context, unlike the other brittle steps in this spec.
Proposed fix
- const providerButtonPresent = await browser.execute(() => { + const providerLabels = await browser.execute(() => { const buttons = Array.from(document.querySelectorAll('button')); - return buttons.some(b => { - const label = b.getAttribute('aria-label') || b.textContent || ''; - return /Google|GitHub|Twitter|Discord/i.test(label); - }); + return buttons + .map(b => (b.getAttribute('aria-label') || b.textContent || '').trim()) + .filter(Boolean); }); + const providerButtonPresent = providerLabels.some(label => + /Google|GitHub|Twitter|Discord/i.test(label) + ); + if (!providerButtonPresent) { + const tree = await dumpAccessibilityTree(); + console.log(`${LOG} Provider buttons not found. Labels:`, providerLabels); + console.log(`${LOG} Tree:\n`, tree.slice(0, 4000)); + } expect(providerButtonPresent).toBe(true);As per coding guidelines "Ensure each E2E spec is runnable in isolation; assert both UI outcomes and backend/mock effects when relevant; add failure diagnostics (request logs,
dumpAccessibilityTree()) for faster debugging".🤖 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 `@app/test/e2e/specs/runtime-picker-login.spec.ts` around lines 267 - 278, Update the "OAuth provider buttons render on Welcome" test to collect and report diagnostics when the provider probe fails: change the probe that sets providerButtonPresent to instead return an object with matched boolean plus arrays of matchedLabels and allButtonLabels (from document.querySelectorAll('button')), and in the test assert on the boolean but on failure log matchedLabels, allButtonLabels and call dumpAccessibilityTree() (or otherwise serialize the DOM/tree) so CI failures include the button labels and accessibility tree for debugging; reference the providerButtonPresent variable and the test name when making the changes.
🤖 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 `@app/test/e2e/specs/runtime-picker-login.spec.ts`:
- Around line 291-295: The check around waitForRequest for '/auth/me' needs to
ensure null fails the test: change the guard to explicitly handle null (using
meCall === null) and call fail() or throw, and update the assertion to
expect(meCall).not.toBeNull() (or expect(meCall).toBeTruthy()) so a null
response does not pass; target the meCall variable returned from
waitForRequest(getRequestLog, 'GET', '/auth/me', 20_000) and include the
existing logging via getRequestLog() when failing.
---
Nitpick comments:
In `@app/test/e2e/specs/runtime-picker-login.spec.ts`:
- Around line 267-278: Update the "OAuth provider buttons render on Welcome"
test to collect and report diagnostics when the provider probe fails: change the
probe that sets providerButtonPresent to instead return an object with matched
boolean plus arrays of matchedLabels and allButtonLabels (from
document.querySelectorAll('button')), and in the test assert on the boolean but
on failure log matchedLabels, allButtonLabels and call dumpAccessibilityTree()
(or otherwise serialize the DOM/tree) so CI failures include the button labels
and accessibility tree for debugging; reference the providerButtonPresent
variable and the test name when making the changes.
🪄 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
Run ID: 24e65d88-8a34-460d-b4bf-831c60a76ee9
📒 Files selected for processing (1)
app/test/e2e/specs/runtime-picker-login.spec.ts
waitForRequest returns null on timeout, which toBeDefined would still pass. toBeTruthy correctly fails the case where the auth profile fetch never happened. (CodeRabbit)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/test/e2e/specs/runtime-picker-login.spec.ts (1)
1-1: ⚖️ Poor tradeoffConsider removing
@ts-nocheckin favor of targeted type fixes or ignores.Blanket suppression can mask real type errors. For
browser.executecallbacks, consider either:
- Adding explicit type parameters to
browser.execute<ReturnType, ArgsType>()- Using targeted
//@ts-expect-error`` on specific lines that resist typingThis improves maintainability as the test evolves.
🤖 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 `@app/test/e2e/specs/runtime-picker-login.spec.ts` at line 1, Remove the top-level "// `@ts-nocheck`" and instead address the specific type issues: replace the blanket suppression with targeted fixes by adding explicit generics to any browser.execute calls (e.g., browser.execute<ReturnType, ArgsType>(...)) and/or apply narrow inline suppressions like "// `@ts-expect-error`" only on the exact lines that cannot be typed; search for browser.execute usages in this spec (runtime-picker-login.spec.ts) and update their signatures with appropriate ReturnType and ArgsType or add line-level comments to preserve type checking elsewhere.
🤖 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.
Nitpick comments:
In `@app/test/e2e/specs/runtime-picker-login.spec.ts`:
- Line 1: Remove the top-level "// `@ts-nocheck`" and instead address the specific
type issues: replace the blanket suppression with targeted fixes by adding
explicit generics to any browser.execute calls (e.g.,
browser.execute<ReturnType, ArgsType>(...)) and/or apply narrow inline
suppressions like "// `@ts-expect-error`" only on the exact lines that cannot be
typed; search for browser.execute usages in this spec
(runtime-picker-login.spec.ts) and update their signatures with appropriate
ReturnType and ArgsType or add line-level comments to preserve type checking
elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9c78dd6-6647-4028-b6d0-535c4a1600b9
📒 Files selected for processing (1)
app/test/e2e/specs/runtime-picker-login.spec.ts
Summary
app/test/e2e/specs/runtime-picker-login.spec.ts(8 cases, ~12 s on Linux Docker) exercising the first-launch login funnel end-to-end.scripts/mock-api-*); deep links go throughwindow.__simulateDeepLinkso the spec is safe on headless Linux containers.Problem
Existing specs cover the auth deep-link path (
login-flow.spec.ts) and a logout-relogin regression (logout-relogin-onboarding.spec.ts), but the first screen — the local-vs-cloud runtime picker — has no e2e coverage. A regression inBootCheckGatecould silently brick onboarding for fresh installs without CI noticing.Solution
e2e/docker-compose.yml), so no new infra.resetCoreMode()), since the e2e build seedsVITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE=localand would otherwise skip it.clickByTextDomwith smallest-descendant scoring,fillInputwith React-aware setter) so tile buttons whosetextContentconcatenates title+description still match.Verified locally on Linux Docker (
docker compose -f e2e/docker-compose.yml run --rm e2e ...): 8 passing, 0 failing.Submission Checklist
Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
Validation Blocked
Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit